Skip to content

HDDS-13621. NPE in OzoneManagerRatisServer.checkRetryCache#9711

Merged
jojochuang merged 1 commit intoapache:masterfrom
ss77892:HDDS-13621
Feb 5, 2026
Merged

HDDS-13621. NPE in OzoneManagerRatisServer.checkRetryCache#9711
jojochuang merged 1 commit intoapache:masterfrom
ss77892:HDDS-13621

Conversation

@ss77892
Copy link
Contributor

@ss77892 ss77892 commented Feb 4, 2026

What changes were proposed in this pull request?

In HDDS-11558, we introduced a retry idempotent. If a retry came from the client, we always want to return the existing reply. But it doesn't cover the case when a failed request landed in the retry cache. In this case, we can't build the response properly, and NPE is thrown.

What is the link to the Apache JIRA

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

How was this patch tested?

ozone freon ommg on the cluster where the OM process is continuously struggling from the JVM pauses. Without the patch, the retry operation never succeeded, so the number of correct writes was always less than expected. With the provided patch, the success rate of retries was always 100%.
I attempted to create a unit test, but I found it quite challenging to replicate the scenario where a pause occurs after the request passes the check for the active leader. As a result, the request ended up in the RetryCache, but it failed when no leader was available.

@ss77892 ss77892 changed the title HDDS-13621 NPE in OzoneManagerRatisServer.checkRetryCache HDDS-13621. NPE in OzoneManagerRatisServer.checkRetryCache Feb 4, 2026
Copy link
Contributor

@cchung100m cchung100m left a comment

Choose a reason for hiding this comment

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

LGTM, thanks to @ss77892

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

lgtm

@jojochuang jojochuang merged commit af22a5c into apache:master Feb 5, 2026
44 checks passed
@jojochuang
Copy link
Contributor

Merged. Thanks @ss77892 for the patch and @cchung100m for review.

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