HBASE-20717 Implement CCSMap - a better concurrent map with compacted data structure#2138
Closed
Reidddddd wants to merge 2 commits intoapache:ccsmapfrom
Closed
HBASE-20717 Implement CCSMap - a better concurrent map with compacted data structure#2138Reidddddd wants to merge 2 commits intoapache:ccsmapfrom
Reidddddd wants to merge 2 commits intoapache:ccsmapfrom
Conversation
Contributor
Author
|
First draft:
|
Contributor
Author
|
In fact, I haven't go through the details yet. I'm thinking let's make v1 draft first, so members can review the details together. |
Reidddddd
commented
Jul 24, 2020
| import java.util.Collections; | ||
| import java.util.Iterator; | ||
| import java.util.List; | ||
| import java.util.Random; |
Contributor
Author
There was a problem hiding this comment.
Oh, I should remove this line...
Reidddddd
commented
Jul 24, 2020
| return theUnsafe.getByte(buf.array(), BYTE_ARRAY_BASE_OFFSET + buf.arrayOffset() + offset); | ||
| } | ||
| } | ||
|
|
Contributor
Author
|
TestCompactedConcurrentSkipList.java passed on local, while TestCompactedTypeHelper failed |
|
💔 -1 overall
This message was automatically generated. |
tedyu
reviewed
Jul 24, 2020
hbase-server/src/test/java/org/apache/hadoop/hbase/ccsmap/TestCompactedConcurrentSkipList.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| @Override | ||
| public KVPair<Integer, String> decomposite(byte[] data, int offset, int len) { |
Contributor
There was a problem hiding this comment.
Looking up the meaning of decomposite, I don't think it reflects the action here.
Was the author intended to say 'decompose' or deserialize ?
hbase-server/src/main/java/org/apache/hadoop/hbase/ccsmap/SpaceNotEnoughExcpetion.java
Outdated
Show resolved
Hide resolved
tedyu
reviewed
Jul 24, 2020
| b.doBehavior(mc); | ||
| long lat = System.nanoTime() - start; | ||
| performance.addOps(b.getName(), lat); | ||
| if (mc.viewOps > 20) { // up to 20 view operations |
Contributor
There was a problem hiding this comment.
I wonder why the reset is called every 20 view operations.
tedyu
reviewed
Jul 24, 2020
| mc.reset(); | ||
| } | ||
| } | ||
| System.out.println("There are " + mc.mainMap2.size() |
Contributor
There was a problem hiding this comment.
Is it possible to establish baseline so that we can make some assertion here ?
I am not sure who would periodically examine these output .
tedyu
reviewed
Jul 24, 2020
| public void testRandomBehaviorParallelly() throws InterruptedException { | ||
| final int RANGE = 32768; | ||
| final int INIT_DATA = 50000; | ||
| final int THREAD = 8; |
Contributor
There was a problem hiding this comment.
Should the thread count be related to number of cores for the processor ?
Contributor
Author
There was a problem hiding this comment.
Somehow what if the vm or poor machine only have 1 processor? although I already changed to availableProcessors methods..
Contributor
Author
|
Hi @tedyu (waving), long time no see! I addressed parts of your reviews, some of them I'm not sure as well. Most of codes can't explain itself, and as we get deep, I believe weird codes may appear more. And I will address them after we have some discussions, like the method decomposite()... |
Contributor
|
It would be good to involve one of the original developers so that we get their buy-in for the changes. Thanks |
Contributor
Author
|
I found that the patch I based on was roughly contributed from Andrew. |
Contributor
Author
|
Temporarily close this PR. |
Contributor
|
Since Alibaba has their own JVM, we should establish benchmark for CCSMap first. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
No description provided.