Skip to content

HDDS-12133. Define replication metrics task names and descriptions as constants to reuse in CommandHandler#9215

Merged
sadanand48 merged 2 commits intoapache:masterfrom
sreejasahithi:HDDS-12133
Nov 21, 2025
Merged

HDDS-12133. Define replication metrics task names and descriptions as constants to reuse in CommandHandler#9215
sadanand48 merged 2 commits intoapache:masterfrom
sreejasahithi:HDDS-12133

Conversation

@sreejasahithi
Copy link
Contributor

What changes were proposed in this pull request?

Currently the commands that go through the replication supervisor are getting the name of the metrics from the task to retrieve from the supervisor, and this may be null if no task has been submitted yet. This results in many null checks in the code.
To solve this the the metrics name and description can be defined as static constants by the task so they can be retrieved directly without null checks.

What is the link to the Apache JIRA

HDDS-12133

How was this patch tested?

Green CI : https://github.com/sreejasahithi/ozone/actions/runs/18874231291

@swamirishi
Copy link
Contributor

@yandrey321 please take a look at it


public String getMetricsName() {
return this.metricsName;
return ECReconstructionCoordinatorTask.METRIC_NAME;

Choose a reason for hiding this comment

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

return task.getMetricName(); ?

public int getQueuedCount() {
return this.metricsName == null ? 0 : (int) this.supervisor
.getReplicationQueuedCount(metricsName);
return (int) this.supervisor.getReplicationQueuedCount(ReconcileContainerTask.METRIC_NAME);

Choose a reason for hiding this comment

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

why specific task type is hardcoded here instead of task.getMetricName() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR replaces the previous approach where handlers needed a task instance to call task.getMetricName(). Instead, I am using static constants from the task classes (TaskClass.METRIC_NAME) so handlers can get metric names without requiring a task instance or null checks. The constants are defined once in each task class and referenced by their respective handlers, eliminating the null checks.

Copy link
Contributor

@sarvekshayr sarvekshayr left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @sreejasahithi.

Below tests still use hardcoded strings instead of referencing the constants:

  • TestReconcileContainerCommandHandler
  • TestReconstructECContainersCommandHandler
  • TestReplicateContainerCommandHandler

Copy link
Contributor

@sadanand48 sadanand48 left a comment

Choose a reason for hiding this comment

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

Thanks @sreejasahithi for the patch, LGTM overall. pending green CI

@sadanand48 sadanand48 merged commit d7e6878 into apache:master Nov 21, 2025
43 checks passed
@sadanand48
Copy link
Contributor

Thanks @sreejasahithi for the contribution , @sarvekshayr , @yandrey321 for the reviews

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.

5 participants