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

Make WorkerInfo enum (and some small refactors) #18460

Merged
merged 7 commits into from
Dec 8, 2023

Conversation

dbw9580
Copy link
Contributor

@dbw9580 dbw9580 commented Dec 6, 2023

What changes are proposed in this pull request?

  1. Move WorkerState enum from master package to the wire package.
  2. Add a new worker state UNRECOGNIZED and use it as the default state.
  3. Make worker state in WorkerInfo an enum.
  4. Add copy constructors to WorkerInfo and WorkerNetAddress.

Why are the changes needed?

  1. Make sure the state of worker can be enumerated.
  2. Allow safely copying mutable WorkerInfo and WorkerNetAddress objects.

Does this PR introduce any user facing changes?

No.

@@ -33,7 +33,7 @@ public final class WorkerInfo implements Serializable {
private WorkerIdentity mIdentity;
private WorkerNetAddress mAddress = new WorkerNetAddress();
private int mLastContactSec;
private String mState = "";
private WorkerState mState = WorkerState.UNRECOGNIZED;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to assign a default value for worker state. A UNRECOGNIZED state means either the code that creates the WorkerInfo object did not set its state explicitly, or the worker is unknown to the membership service (e.g. never registered).

@jiacheliu3 jiacheliu3 changed the title Refactor worker info Make WorkerInfo enum (and some small refactors) Dec 7, 2023
Copy link
Contributor

@jiacheliu3 jiacheliu3 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

/***
* The worker state maintained by master.
*/
public enum WorkerState {
Copy link
Contributor

Choose a reason for hiding this comment

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

anchor: moved from dora/core/common/src/main/java/alluxio/master/WorkerState.java

Comment on lines +83 to +88
if (Modifier.isStatic(fieldModifiers) || Modifier.isFinal(fieldModifiers)) {
continue;
}
field.setAccessible(true);
// set fields in the copy to their default value
field.set(copied, Defaults.defaultValue(field.getType()));
Copy link
Contributor

Choose a reason for hiding this comment

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

lol, works for me

@dbw9580 dbw9580 added the type-code-quality code quality improvement label Dec 8, 2023
@dbw9580
Copy link
Contributor Author

dbw9580 commented Dec 8, 2023

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 73d1746 into Alluxio:main Dec 8, 2023
14 checks passed
ssz1997 pushed a commit to ssz1997/alluxio that referenced this pull request Dec 15, 2023
### What changes are proposed in this pull request?

1. Move `WorkerState` enum from master package to the wire package.
2. Add a new worker state `UNRECOGNIZED` and use it as the default state.
3. Make worker state in `WorkerInfo` an enum.
4. Add copy constructors to `WorkerInfo` and `WorkerNetAddress`.

### Why are the changes needed?

1. Make sure the state of worker can be enumerated.
2. Allow safely copying mutable `WorkerInfo` and `WorkerNetAddress` objects.

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

No.

			pr-link: Alluxio#18460
			change-id: cid-8daf9c1e3ebe8e862a9b0dabb669c80918f5b8b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-code-quality code quality improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants