Skip to content

fix: return -1 from getWorkerIndex for non-worker actor IDs#5006

Open
Ma77Ball wants to merge 2 commits intoapache:mainfrom
Ma77Ball:fix/getWorkerIndexMatchError
Open

fix: return -1 from getWorkerIndex for non-worker actor IDs#5006
Ma77Ball wants to merge 2 commits intoapache:mainfrom
Ma77Ball:fix/getWorkerIndexMatchError

Conversation

@Ma77Ball
Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

VirtualIdentityUtils.getWorkerIndex only matched the worker name pattern with no fallback case, so passing a non-worker ActorVirtualIdentity (e.g. CONTROLLER, SELF) threw scala.MatchError at runtime. This PR adds a fallback case that returns -1 for non-worker actor IDs, matching the graceful handling already present in the sibling methods getPhysicalOpId and toShorterString.

Any related issues, documentation, discussions?

Closes: #4727

How was this PR tested?

Updated VirtualIdentityUtilsSpec the existing test that pinned the MatchError behavior was replaced with a test asserting that getWorkerIndex returns -1 for special actor IDs like CONTROLLER. The existing test for the worker-name happy path still passes unchanged.

Was this PR authored or co-authored using generative AI tooling?

Co-authored with Claude Opus 4.7 in compliance with ASF

@Ma77Ball Ma77Ball changed the title return -1 from getWorkerIndex for non-worker actor IDs instead of thr… fix: return -1 from getWorkerIndex for non-worker actor IDs May 10, 2026
@Ma77Ball
Copy link
Copy Markdown
Contributor Author

/request-review @Yicong-Huang

@github-actions github-actions Bot requested a review from Yicong-Huang May 10, 2026 03:29
@chenlica
Copy link
Copy Markdown
Contributor

@Ma77Ball Let's try to assign PRs to other committers.

@Xiao-zhen-Liu Please review it.

@chenlica chenlica requested review from Xiao-zhen-Liu and removed request for Yicong-Huang May 10, 2026 04:49
@Ma77Ball
Copy link
Copy Markdown
Contributor Author

@chenlica I've also used @aicam and @kunwp1. However, I get your point. I have used @aglinxinyuan and @Yicong-Huang a lot recently. Given the large codebase, it can be hard to know who to ask for PR reviews. It would be good to either:

  1. Have suggestions (automatically generated based on which parts of the codebase were edited)
  2. Descriptions of each committer and which parts of the codebase they are most comfortable with (a reference for contributors to use the /request-review feature manually).

I can raise an issue if you think this is an idea the project would like to pursue.

@Yicong-Huang
Copy link
Copy Markdown
Contributor

Let's not make PR triage too complicated for now. We can have auto suggestions as an improvement down the road, but for now it's easier and better to do manual triage: If a requested reviewer knows who have a better context, simply reassign.

Also, our code base is tiny, compared to other projects. You can usually do a git blame to find the owner/author of a component.

If not, feel free to request @chenlica and he can find ppl to review.

Let's not be blocked by some fancy auto triage method.

@Yicong-Huang
Copy link
Copy Markdown
Contributor

Yicong-Huang commented May 10, 2026

In addition, I highly suggest everyone to review more PRs, even if that's not your experty. Share your thoughts, learn the code base, ask questions.

For PR authors, feel free to request more ppl to get their attention.

Descriptions of each committer and which parts of the codebase they are most comfortable with (a reference for contributors to use the /request-review feature manually).

Everyone should be comfortable review all codebase. Don't own only a file, or two. Don't be shy. I encourage everyone (not just committers) to contribute to and review everywhere.

@chenlica
Copy link
Copy Markdown
Contributor

@Ma77Ball It will be easier for me to assign people to review PRs. Yes, we need more members to review PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VirtualIdentityUtils.getWorkerIndex throws MatchError on non-worker actor names

3 participants