-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-24735: Refactor ReplicationSourceManager: move logPositionAndCl… #2064
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
Conversation
|
💔 -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. |
bharathv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice cleanup, minor comments and then +1
...c/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java
Outdated
Show resolved
Hide resolved
...server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
Show resolved
Hide resolved
| entryBatch.getLastWalPosition(), entryBatch.getLastSeqIds())); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this to the interface?
...src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryBatch.java
Outdated
Show resolved
Hide resolved
27dab4e to
6f28e3b
Compare
|
🎊 +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. |
|
🎊 +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. |
| protected final ConcurrentHashMap<String, ReplicationSourceShipper> workerThreads = | ||
| new ConcurrentHashMap<>(); | ||
|
|
||
| private AtomicLong totalBufferUsed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are cleaning out this extra, unnecessary variable, could we make "this.manager" package private (or provide a package private getter), so that source readers and source shippers don't have to define an additional reference to ReplicationSourceManager?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Let me refator it. And after I rethink about this, the ReplicationSourceManager is still needed by ReplicationSource. But only need the getTotalBuffered/getTotalLimit/getGlobalMetrics now. Maybe introduce a new interface in the future.
...server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
Outdated
Show resolved
Hide resolved
...server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
Show resolved
Hide resolved
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside
|
🎊 +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. |
wchevreuil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest UT failure seems unrelated. +1
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (apache#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (#2064) Signed-off-by: Wellington Chevreuil <[email protected]>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside