adoroszlai commented on code in PR #6742:
URL: https://github.com/apache/ozone/pull/6742#discussion_r1630682975
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconPipelineManager.java:
##########
@@ -166,10 +166,14 @@ public void removeInvalidPipelines(List<Pipeline>
pipelinesFromScm) {
* @throws IOException
*/
@VisibleForTesting
- public void addPipeline(Pipeline pipeline)
- throws IOException {
+ public void addPipeline(Pipeline pipeline) throws IOException {
acquireWriteLock();
try {
+ // Check if the pipeline already exists
+ if (containsPipeline(pipeline.getId())) {
+ LOG.warn("Duplicate pipeline ID {} detected, skipping addition.",
pipeline.getId());
+ return;
+ }
getStateManager().addPipeline(
pipeline.getProtobufMessage(ClientVersion.CURRENT_VERSION));
Review Comment:
This method has three non-test callers, and all three handle this case
differently:
1. add or update:
https://github.com/apache/ozone/blob/9e0f9677d497ac444bcee029a593c27c3a65f989/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconPipelineManager.java#L107-L120
2. add if not exists:
https://github.com/apache/ozone/blob/9e0f9677d497ac444bcee029a593c27c3a65f989/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java#L233-L237
3. add always:
https://github.com/apache/ozone/blob/9e0f9677d497ac444bcee029a593c27c3a65f989/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconPipelineReportHandler.java#L86-L88
So two of these already call `containsPipeline` before calling
`addPipeline`. For these the additional check is unnecessary.
Also, should it apply the "add or update" logic in all 3 calls, to ensure
pipeline information is current?
--
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]
