Support StressWorkerBench using consistent hash policy #18246
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jiacheliu3
requested changes
Oct 9, 2023
dora/core/client/fs/src/main/java/alluxio/client/file/dora/WorkerLocationPolicy.java
Outdated
Show resolved
Hide resolved
Comment on lines
100
to
105
| @Parameter(names = {"--mode"}, | ||
| description = "Decide which policy to use for file reads." | ||
| // + "Possible values are: [local-only, remote-only, hash]." | ||
| + "Possible values are: [local-only, hash]." | ||
| + "By default, the mode is 'hash'") | ||
| public String mMode = "hash"; |
There was a problem hiding this comment.
you can use a enum here and the values can then be LOCAL_ONLY, HASH
Let's only support LOCAL_ONLY and HASH. We can add a REMOTE_ONLY when we implement that policy.
Here the description can be
description = "Specifies which worker the test process reads from."
+ "Possible values are: [" + Arrays.toString(YourEnumClass.values()) + "]"
+ "HASH - alluxio.client.file.dora.ConsistentHashPolicy"
+ "LOCAL_ONLY - alluxio.client.file.dora.LocalWorkerPolicy"
+ "The default is HASH."
You may replace the strings with enum/class names by code, as you see fit.
There was a problem hiding this comment.
Description should be a constant string, or it will report: Attribute value must be constant. Here I will use a string.
dora/stress/shell/src/main/java/alluxio/stress/cli/worker/StressWorkerBench.java
Outdated
Show resolved
Hide resolved
dora/stress/shell/src/main/java/alluxio/stress/cli/worker/StressWorkerBench.java
Outdated
Show resolved
Hide resolved
jiacheliu3
requested changes
Oct 9, 2023
dora/core/client/fs/src/main/java/alluxio/client/file/dora/WorkerLocationPolicy.java
Outdated
Show resolved
Hide resolved
| + "HASH - alluxio.client.file.dora.ConsistentHashPolicy" | ||
| + "LOCAL_ONLY - alluxio.client.file.dora.LocalWorkerPolicy" | ||
| + "The default is HASH.") | ||
| public String mMode = "HASH"; |
There was a problem hiding this comment.
can you use enum here instead of a string? Refer to my previous comment on how.
| @@ -191,16 +191,18 @@ public void prepare() throws Exception { | |||
| "true"); | |||
|
|
|||
| // default mode value: hash, using consistent hash | |||
| if (Objects.equals(mParameters.mMode, "local-only")) { | |||
| // TODO(jiacheng): we may need a policy to only IO to remote worker | |||
| if (Objects.equals(mParameters.mMode, "LOCAL_ONLY")) { | |||
There was a problem hiding this comment.
here mParameters.mMode should be an enum so you just
switch (mParameters.mMode) {
case LOCAL_ONLY: ...; break;
case HASH: ...;break;
default: throw new IllegalArgumentException("Unrecognized mode" + mParameters.mMode);
}
jiacheliu3
approved these changes
Oct 10, 2023
jiacheliu3
changed the title
|
alluxio-bot, merge this please |
ssz1997
pushed a commit
to ssz1997/alluxio
that referenced
this pull request
Dec 15, 2023
### What changes are proposed in this pull request? This PR adds a `--mode` option for StressWorkerBench. This option allows user to choose from a range of file read policies. Possible option values are `hash`(default) and `local-only`. If use `hash`, the config `alluxio.client.file.dora.ConsistentHashPolicy` is set to `alluxio.client.file.dora.ConsistentHashPolicy`. If use `local-only`, that config is set to `alluxio.client.file.dora.LocalWorkerPolicy`. The benchmark will parse this option, and choose the right policy. Also, it will print a log about the policy the user is using in both the benchmark and the policy factory. ### Why are the changes needed? In previous versions, the default policy is `local-only`. However, in Alluxio, the default policy is `hash`, using `alluxio.client.file.dora.ConsistentHashPolicy`. If user wants to use the `local-only` policy, add `--mode local-only` to the end of the benchmark command. pr-link: Alluxio#18246 change-id: cid-29676a12710b9c815f3e55f84eb5e4226fea9ad7
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
This PR adds a
--modeoption for StressWorkerBench. This option allows user to choose from a range of file read policies. Possible option values arehash(default) andlocal-only.If use
hash, the configalluxio.client.file.dora.ConsistentHashPolicyis set toalluxio.client.file.dora.ConsistentHashPolicy.If use
local-only, that config is set toalluxio.client.file.dora.LocalWorkerPolicy.The benchmark will parse this option, and choose the right policy. Also, it will print a log about the policy the user is using in both the benchmark and the policy factory.
Why are the changes needed?
In previous versions, the default policy is
local-only.However, in Alluxio, the default policy is
hash, usingalluxio.client.file.dora.ConsistentHashPolicy.If user wants to use the
local-onlypolicy, add--mode local-onlyto the end of the benchmark command.