HDFS-15202 Boost short circuit cache #1884
Closed
Conversation
This file contains hidden or 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
pustota2009
changed the title
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
reviewed
Mar 27, 2020
...oject/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
reviewed
May 12, 2020
| @@ -141,6 +141,8 @@ | |||
| "dfs.short.circuit.shared.memory.watcher.interrupt.check.ms"; | |||
| int DFS_SHORT_CIRCUIT_SHARED_MEMORY_WATCHER_INTERRUPT_CHECK_MS_DEFAULT = | |||
| 60000; | |||
| String DFS_SHORT_CIRCUIT_NUM = "dfs.short.circuit.num"; | |||
There was a problem hiding this comment.
can i ask you to change this to dfs.client.short.circuit.num? all client configs starts with dfs.client
There was a problem hiding this comment.
if you change, please also update hdfs-default.xml
There was a problem hiding this comment.
@jojochuang thank you for interesting! Done)
| @@ -227,8 +237,8 @@ public String getConfString() { | |||
| return confString; | |||
| } | |||
|
|
|||
| public ShortCircuitCache getShortCircuitCache() { | |||
| return shortCircuitCache; | |||
| public ShortCircuitCache getShortCircuitCache(long idx) { | |||
There was a problem hiding this comment.
This is fine. But what I realized is Hadoop applications don't really respect the annotation @InterfaceAudience.Private. You really want to gracefully deprecate a public method to avoid runtime issues. That is, keep the original getShortCircuitCache() method and let it call getShortCircuitCache(0);
There was a problem hiding this comment.
That is, keep the original getShortCircuitCache() method and let it call
Sure, done
|
Take a look please at this UT - is it ok? For example if clientShortCircuitNum is 3, when a lot of blockids of SSR are ***1, ***4, ***7， this situation will fall into a ShortCircuitCache. |
|
@pustota2009 |
rename dfs.short.circuit.num to dfs.client.short.circuit.num
return back getShortCircuitCache()
I understand what do you mean, thank you for explanation. We talked about this and agreed that we can go step by step. Could you merge this PR and I promise to do some research about other strategy like crc32 later? |
|
💔 -1 overall
This message was automatically generated. |
added comment
|
💔 -1 overall
This message was automatically generated. |
refact
|
💔 -1 overall
This message was automatically generated. |
|
@pustota2009 can you rebase the patch? |
|
@jojochuang of course, just created new PR |
|
actually, you just need to rebase, resolve the conflcits and push the new commits to your branch. No need to start a new PR. But let's move the discussion there. |
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.
Improve HDFS-client massive reading performance.
The idea: create few instances ShortCircuit caches instead of one.
It helps avoid locks and lets CPU do job.