saintstack commented on pull request #2413:
URL: https://github.com/apache/hbase/pull/2413#issuecomment-694982779
Looks like I misunderstood you then. I apologize. (Having trouble on when
and when not to R***).
`So to simplify the implementation, I suggest that first we could just
create hbase meta source everytime(only controlled by the meta read replicas
enabled flag). And in Replication.init, agree that we could pass a WALFactory,
and we could create a WALActionListener in the init method and register it to
WALFactory, and when we actually create meta provider, we register the listener
to the meta provider.`
The configuration will be everywhere on all servers. You are suggesting
create MetaReplicationSource on all servers on startup even though a server may
not carry an hbase:meta Region? And a WALFileLengthProvider for meta on
Replication init... Keep them around to hook to the MetaWALProvider when it is
lazily instantiated?
(Other questions, do I use a ReplicationSourceFactory creating this
MetaReplicationSource...If so, how without mixing in the as-yet not-created
WALProvider/WALFactory)
`And on the ReplicationSource implementation, after reviewing the
replication related code, I think we still need to have
ReplicationSourceManager for meta replication source, as lots of other classes
such as ReplicationSourceWALReader need it. And the only thing effects us is
the logPositionAndCleanOldLogs method, it is called in
ReplicationSourceShipper. So in general, at least we need to hack this method
to treat meta replication source specially.`
So, MetaReplicationSource in ReplicationSourceManager as a special data
member rather than as part of the general map of ReplicationSources?
The WALFileLengthProvider is currently supplied by WALProvider. Making it so
WALFileLengthProvider instead figures which WALProvider it is backed by is an
inversion.
'Anyway, I still prefer that we have a dummy ReplicationQueueStorage so the
logic could be simpler.`
So, rather than ask the ReplicationSource if it needs to persist, instead,
persist everything but have the MetaReplicationSource persistence be a noop?
`
And after rethinking, if we want to support splittable meta(no matter which
way we choose), then add/remove meta source when assign/unassign meta region
will be much complicated, as we need to do reference counting and there could
be race as the design of assign/unassign is that different regions can run
concurrently.`
I think this shouldn't be too bad if the adding/removing goes via
ReplicationSourceManager. It will know when it is ok to shut down the
ReplicationSource.
Let me work on making this patch smaller. Will break out pieces. If you get
a chance, would appreciate answers on the above. Will also work on the
suggestions to see if they simplify the implementation.
I asked if there a writeup on high-level needs around this area of the code
that I may have missed but perhaps there isn't one? This seems to be one
objective:
`... I think why we introduce a WALFileLengthProvider is that we do not
want to reference WALFactory and WALProvider directly in the replication code
so in the future it will be easier to decouple them, but in this PR we add both
WALFactory and WALProvider references to the replication code, which is not
good...`
Are there others you can think of?
We did a design for the work in this PR. Perhaps we could get your needs
up in the design doc?
Thanks
----------------------------------------------------------------
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]
