HBASE-25715 Add a basic CompactionServer which only keep heartbeat with HMaster#3115
HBASE-25715 Add a basic CompactionServer which only keep heartbeat with HMaster#3115nyl3532016 wants to merge 4 commits intoapache:HBASE-25714from
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. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Apache9
left a comment
There was a problem hiding this comment.
Skimmed, the code for HCompactionServer is very like HRegionServer, just some naming changes...
Could we try to reduce the duplicated code? By introducing a common base class, or a util class?
Thanks.
There was a problem hiding this comment.
I believe we have this method at ohter places too, is it possible to reuse the code instead of copy the code?
There was a problem hiding this comment.
Yes, Let me try to reduce the code
33baad5 to
a5ea930
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. |
There was a problem hiding this comment.
What's this class used for?
There was a problem hiding this comment.
Just like RegionServerServices, useless in this patch. Remove first, I will add this class later.
There was a problem hiding this comment.
I think this only difference here between the similiar method in HRegionServer is how to call newStub? We need to use different types of protobuf Interface. So can we move this method up to AbstractServer?
There was a problem hiding this comment.
Abstract a method createMasterStub, the original return value master serverName is only used for debug, so return actual stub created instead.
| // server manager to deal with region server info | ||
| private volatile ServerManager serverManager; | ||
|
|
||
| private volatile CompactionServerManager compactionServerManager; |
There was a problem hiding this comment.
Is it possible to reuse ServerManager here? Just asking...
There was a problem hiding this comment.
I think no need now, has few duplicated codes. CompactionServer do not register to zookeeper, so compactionServerManager is more simple without zookeeper interaction.
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@Apache9 Would you help review it? Thanks |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
precommit checks / yetus jdk11 hadoop3 checks failed due to workspace read only, I request a new PR |

No description provided.