Skip to content

HDDS-5324. Delete unnecessary calls to decNumKeys in OMKeysDeleteRequest#2319

Merged
bharatviswa504 merged 1 commit intoapache:masterfrom
wzhallright:HDDS-5324
Jun 14, 2021
Merged

HDDS-5324. Delete unnecessary calls to decNumKeys in OMKeysDeleteRequest#2319
bharatviswa504 merged 1 commit intoapache:masterfrom
wzhallright:HDDS-5324

Conversation

@wzhallright
Copy link
Contributor

@wzhallright wzhallright commented Jun 9, 2021

What changes were proposed in this pull request?

If you fail to delete keys, you can avoid unnecessary calls to decNumKeys.

What is the link to the Apache JIRA

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

How was this patch tested?

}
break;
case FAILURE:
omMetrics.decNumKeys(deleteKeys.size());
Copy link
Contributor

Choose a reason for hiding this comment

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

// reset deleteKeys as request failed.
deleteKeys = new ArrayList<>();

So, even without this fix also I don't see an issue. Have you observed any issue?
Or you want to avoid unnecessary call to decNumKeys is this the reason for PR?

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 for review, sorry for i overlooked it in catch.
If in order to avoid unnecessary calls to decNumKeys, is it necessary to delete it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will modify the title if necessary, otherwise I will close this pr.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove that unnecessary call. I am fine with it. You can update the Jira description.

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, PTAL!

@wzhallright wzhallright changed the title HDDS-5324. Shouldn't decNumKeys when delete keys fail HDDS-5324. Delete unnecessary calls to decNumKeys in OMKeysDeleteRequest Jun 9, 2021
Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@bharatviswa504
Copy link
Contributor

This just removes unnecessary codeine, and test failures are unrelated to the patch.
Going ahead with merge to kick unnecessary another CI run to save CI resources.

@bharatviswa504 bharatviswa504 merged commit 838d5d8 into apache:master Jun 14, 2021
@bharatviswa504
Copy link
Contributor

Thank you @wzhallright for the contribution.

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.

2 participants