HDDS-6895. EC: ReplicationManager - Logic to process the under replicated queue and assign work to DNs - #3599
Conversation
…ECContainersCommand
| type = ConfigType.TIME, | ||
| defaultValue = "30s", | ||
| tags = {SCM, OZONE}, | ||
| description = "Hpw frequently to check if there are work to process " + |
umamaheswararao
left a comment
umamaheswararao
left a comment
There was a problem hiding this comment.
Overall patch looks good to me.
There is a typo comment to address, otherwise let's push this.
I feel we need some integration tests around this. But we can add in followup JIRA. Let me file one.
|
|
||
| private final long containerID; | ||
| private final List<DatanodeDetails> sourceDatanodes; | ||
| private int replicaIndex = 0; |
There was a problem hiding this comment.
We discussed offline about alternative approach to get the replicaIndex. I am updating here for the discussion context.
We discussed that this may be ok and simpler, even though we are not using this at the DN side currently. We could potentially use this for validation purposes if needed.
The alternative method is that we could add a methods at ContainerManager to get the replicaIndex and we can use that method to get replicaIndex for a given node. After discussion we felt this is ok to just add to command rather than going back to container manager.
adoroszlai
left a comment
adoroszlai
left a comment
There was a problem hiding this comment.
LGTM, only noticed some more typos.
| ecReplicationConfig, HddsIdFactory.getLongId()); | ||
| } | ||
|
|
||
| // Should be called only for protobuf conversion |
There was a problem hiding this comment.
Nit: comment seems to be outdated, as now it's called from the other constructor.
| * into maintenance. | ||
| * @return Returns the key value pair of destination dn where the command gets | ||
| * executed and the command itself. | ||
| * executed and the command itself. If an empty list if returned, it indicates |
There was a problem hiding this comment.
| * executed and the command itself. If an empty list if returned, it indicates | |
| * executed and the command itself. If an empty list is returned, it indicates |
| private final ReentrantLock lock = new ReentrantLock(); | ||
| private Queue<ContainerHealthResult.UnderReplicatedHealthResult> | ||
| underRepQueue; | ||
| private ECUnderReplicationHandler ecUnderReplicationHandler; |
There was a problem hiding this comment.
Can be final?
| private ECUnderReplicationHandler ecUnderReplicationHandler; | |
| private final ECUnderReplicationHandler ecUnderReplicationHandler; |
| * into maintenance. | ||
| * @return Returns the key value pair of destination dn where the command gets | ||
| * executed and the command itself. | ||
| * executed and the command itself. If an empty list if returned, it indicates |
There was a problem hiding this comment.
| * executed and the command itself. If an empty list if returned, it indicates | |
| * executed and the command itself. If an empty list is returned, it indicates |
…replicated queues and assign work to DNs HDDS-6895. EC: ReplicationManager - Logic to process the under replicated queue and assign work to DNs (apache#3599) (cherry picked from commit 9ebb4aa) Change-Id: I88836a439f5526c6e33e4e54df9797004bc2418a
3 participants
What changes were proposed in this pull request?
We need some sort of thread which picks work from the under / over replicated queue and assigns the work to DNs with capacity. The DNs will pick the work up on each heartbeat.
This initial change does not consider DN capacity or any limits. It will process all under replicated EC containers and assign them to the DN.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6895
How was this patch tested?
New unit tests added