Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix free worker command bugs #16458

Merged
merged 5 commits into from
Feb 22, 2023

Conversation

YichuanSun
Copy link
Contributor

@YichuanSun YichuanSun commented Nov 2, 2022

What changes are proposed in this pull request?

Fix potential bugs in freeWorker command.

Why are the changes needed?

When a worker has been decommissioned, its metadata can not be got by calling getWorkerInfolist(). This method accesses LoadingCache<String, List<WorkerInfo>> mWorkerInfoCache in DefaultBlockMaster.java, which will not refresh instantly.

As to method removeDecommissionedWorker() in BlockMasterClientServiceHandler.java, if we don't add FieldRanges, the list decommissionedWorkers would not get enough information to run the loop below successfully, though the worker has been decommissioned.

Does this PR introduce any user facing changes?

No.

Copy link
Contributor

@maobaolong maobaolong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It LGTM overall.

Would you like to add a space after "Worker" in L87

 System.out.println("Worker" + workerName + " is not found in decommissioned worker set.");

@YichuanSun
Copy link
Contributor Author

It LGTM overall.

Would you like to add a space after "Worker" in L87

 System.out.println("Worker" + workerName + " is not found in decommissioned worker set.");

Thanks for your comment! I have added the space. Can you take a look at Fix a bug in WorkerInfoField? Because the BLOCK_COUNT in WorkerInfoField is added by you.

Copy link
Contributor

@dbw9580 dbw9580 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Add comments in FreeWorker command.

Co-authored-by: Bowen Ding <6999708+dbw9580@users.noreply.github.com>
@LuQQiu LuQQiu added area-shell Alluxio Command Line Interface area-worker Alluxio worker labels Feb 4, 2023
@jiacheliu3 jiacheliu3 closed this Feb 21, 2023
@jiacheliu3 jiacheliu3 reopened this Feb 21, 2023
@jiacheliu3
Copy link
Contributor

alluxio-bot, merge this please

@alluxio-bot
Copy link
Contributor

Check evaluation failed:
Github action modules: (alluxio.server.ft.,!alluxio.server.ft.journal.raft.,!alluxio.server.ft.journal.ufs... concluded as failure, which is not success or skipped

@YichuanSun
Copy link
Contributor Author

All checks are green below but checks panel shows that the modules: (alluxio.server.ft.,!alluxio.server.ft.journal.raft.,!alluxio.server.ft.journal.ufs... in Java 8 Integration Tests is failed. Can you run the test again? Thanks. @jiacheliu3

@jiacheliu3 jiacheliu3 closed this Feb 22, 2023
@jiacheliu3 jiacheliu3 reopened this Feb 22, 2023
@jiacheliu3
Copy link
Contributor

jiacheliu3 commented Feb 22, 2023

Re-opened to trigger all tests again. If the bot still doesn't recognize, i will force merge.

@jiacheliu3
Copy link
Contributor

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit ff0a6da into Alluxio:master Feb 22, 2023
YangchenYe323 pushed a commit to YangchenYe323/alluxio that referenced this pull request Apr 16, 2023
### What changes are proposed in this pull request?

Fix potential bugs in freeWorker command.

### Why are the changes needed?

When a worker has been decommissioned, its metadata can not be got by
calling `getWorkerInfolist()`. This method accesses
`LoadingCache<String, List<WorkerInfo>> mWorkerInfoCache` in
`DefaultBlockMaster.java`, which will not refresh instantly.

As to method `removeDecommissionedWorker()` in
`BlockMasterClientServiceHandler.java`, if we don't add FieldRanges, the
list `decommissionedWorkers` would not get enough information to run the
loop below successfully, though the worker has been decommissioned.

### Does this PR introduce any user facing changes?

No.

pr-link: Alluxio#16458
change-id: cid-101865dc9ec4f40e7561f81f38287c6efc2ae23f
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 16, 2023
### What changes are proposed in this pull request?

Fix potential bugs in freeWorker command.

### Why are the changes needed?

When a worker has been decommissioned, its metadata can not be got by
calling `getWorkerInfolist()`. This method accesses
`LoadingCache<String, List<WorkerInfo>> mWorkerInfoCache` in
`DefaultBlockMaster.java`, which will not refresh instantly.

As to method `removeDecommissionedWorker()` in
`BlockMasterClientServiceHandler.java`, if we don't add FieldRanges, the
list `decommissionedWorkers` would not get enough information to run the
loop below successfully, though the worker has been decommissioned.

### Does this PR introduce any user facing changes?

No.

pr-link: Alluxio#16458
change-id: cid-101865dc9ec4f40e7561f81f38287c6efc2ae23f
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 16, 2023
### What changes are proposed in this pull request?

Fix potential bugs in freeWorker command.

### Why are the changes needed?

When a worker has been decommissioned, its metadata can not be got by
calling `getWorkerInfolist()`. This method accesses
`LoadingCache<String, List<WorkerInfo>> mWorkerInfoCache` in
`DefaultBlockMaster.java`, which will not refresh instantly.

As to method `removeDecommissionedWorker()` in
`BlockMasterClientServiceHandler.java`, if we don't add FieldRanges, the
list `decommissionedWorkers` would not get enough information to run the
loop below successfully, though the worker has been decommissioned.

### Does this PR introduce any user facing changes?

No.

pr-link: Alluxio#16458
change-id: cid-101865dc9ec4f40e7561f81f38287c6efc2ae23f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-shell Alluxio Command Line Interface area-worker Alluxio worker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants