Skip to content

Conversation

@ivandika3
Copy link
Contributor

What changes were proposed in this pull request?

Since HadoopRpcOMFollowerReadFailoverProxyProvider handles only submitRequest from OzoneManagerProtocolPB, we should simply make it to only handle OzoneManagerProtocolPB.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14490

How was this patch tested?

CI (https://github.com/ivandika3/ozone/actions/runs/21347454675)

@ivandika3 ivandika3 requested a review from szetszwo January 26, 2026 07:43
@ivandika3 ivandika3 self-assigned this Jan 26, 2026
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ivandika3 for the patch.

@@ -107,32 +95,25 @@ public HadoopRpcOMFollowerReadFailoverProxyProvider(String omServiceId, Class<T>
.map(a -> a.proxyInfo)
.reduce((a, b) -> a + ", " + b).orElse("") + "]";
@SuppressWarnings("unchecked")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppression is no longer needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, removed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (not introduced in this patch): This is currently unused. Is this going to change in follow-up or can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this first. We can add this when we need it.

Comment on lines 64 to 65
@VisibleForTesting
public static final Logger LOG = LoggerFactory.getLogger(HadoopRpcOMFollowerReadFailoverProxyProvider.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (not introduced in this patch): this can be private and annotation removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated.

public HadoopRpcOMFollowerReadFailoverProxyProvider(String omServiceId, Class<T> protocol,
HadoopRpcOMFailoverProxyProvider<T> failoverProxy) {
this.protocolClass = protocol;
public HadoopRpcOMFollowerReadFailoverProxyProvider(String omServiceId,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: omServiceId is unused

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, removed.

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 the change looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants