saintstack commented on pull request #2413: URL: https://github.com/apache/hbase/pull/2413#issuecomment-694634837
``` Yes, let's do the refactoring. We have a queueStorage in ReplicationSource, so in ReplicationSourceManager, if we have a ReplicationSource when we want to store or load something from queueStorage, we could use the one in the ReplicationSource. For other place such as adoptAbandonedQueues, we still use the global queueStorage. I think this could make our code cleaner. I think this can be done before this PR, as a separated issue, to make the PR here smaller and easier to review. ``` Let me give this a go. `Let's not talk about root yet? It has not been implemented in HBase yet. Since we have a special 'metaProvider' field in WALFactory, why not also have a special 'metaSource' in ReplicationSourceManager? I do not think force mixing two completely different things is a good idea here. For example, for metaSource we only care about the wal files from metaProvider but if you add it to the sources map, it will also receive the wal files from other wal providers, then you need extra logic to filter them out. For a special logic, I prefer we also give it a special code path.` ROOT is floating about these times but I can pretend it is not if it helps this discussion. So we have a user-space WALProvider and a Meta WALProvider. There will be no more WALProvider types? We won't want to read WALs from other than HDFS for instance? So, you are arguing against hooking a WALProvider up to a ReplicationSource? If I don't add it to the source map, then I have to do special-casing and accounting for the meta path. `Got your point but I have a different idea here. In general I think the current approach is good, we'd better reuse the current replication framework as much as we can. But for me, I would like to just reuse the 'replication' code, but not the replication source management code. That means, we should reuse most code in ReplicationSource(as you have one in this PR), but better to have a special code path in ReplicationSourceManager to add/remove the replication source, to receive new wal files etc, as I do not think it will add too much code, and could leave the original code as is to not confuse developpers.` Let me try and understand where you are going so I can try it. You are suggesting that I'd create a MetaReplicationSource on hbase:meta open (and remove it on close) and not register it in the ReplicationSource Map. I'd not create it NOT using ReplicationSourceFactory. I'd not use ReplicationSourceManager. All other ReplicationSources would only ever take the user-space WALProvider. Do I have it right? `And in HBASE-24950, I do have a framework to do root replication without using the replication framework, it is much simpler but highly specialized for catalog family only. You can take a look later after I finished a POC but I do not think it suits here. As in general, I think we'd better also reimplement the normal read replica implementation to not store the replication queue, it is not necessary.` Yeah. I've seen this. I don't think this a good idea having Master serving Region location data. If others agree, we'll be looking for a way to do region replicas for ROOT region --wherever it is hosted -- and we'll be back in here to add it. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
