sodonnel commented on PR #3384: URL: https://github.com/apache/ozone/pull/3384#issuecomment-1123886620
This change feels like we are moving a lot of pieces from LegacyReplicationManager into InflightActionManager.java, and I am not sure if they all belong in that new class either. I had imagined creating a small "InflightAction" class that is a wrapper around the maps used to store the inflight actions, and then allow that to be shared by both Replication Managers. However I am not even sure on that now, as the Legacy manager will deal with a different set of containers than the new manager. There is a lot of logic in the Legacy RM that is bound tightly to the inflightActions map and its not clear if we will want to carry that over into the new one, or do things in a slightly different way. I wonder if we may get a better result if we start building up the new replication manager piece by piece and then see if we get something a little cleaner. We would also avoid excessive changes to the LegacyManager for now until we have a better idea about how the new ReplicationManager looks. What do you think? -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
