Skip to content

Conversation

@ChenSammi
Copy link
Contributor

@ChenSammi ChenSammi commented Apr 1, 2024

What changes were proposed in this pull request?

OzoneManager crashed during reapply Ratis transactions during startup. During recoverLease reapply, the request wants to generate the block token which depends on SecretKey service, while SecretKey client is not instantiated at that time because OzoneManager is reapply txs, and all the other services are not yet initialized.

What is the link to the Apache JIRA

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

How was this patch tested?

Tested in the cluster where the issue is found and can repeatedly reproduced.

@jojochuang jojochuang added the hbase HBase on Ozone support label Apr 2, 2024

OMClientResponse omClientResponse = validateAndUpdateCache();
OMResponse omResponse = omClientResponse.getOMResponse();
assertEquals(OzoneManagerProtocolProtos.Status.OK, omResponse.getStatus());
Copy link
Contributor

Choose a reason for hiding this comment

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

is it okay to just let it return OK? Or is it better to return an error? Ideally the client should keep retrying until OM is running.

Copy link
Contributor Author

@ChenSammi ChenSammi Apr 3, 2024

Choose a reason for hiding this comment

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

This path is only for ratis raft log reapply, so skip the token is OK. But I find a more clean way to handle this. I will upload a new patch.

Copy link
Contributor Author

@ChenSammi ChenSammi Apr 3, 2024

Choose a reason for hiding this comment

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

The new patch is tested by Pratyush, and it works, the issue doesn't happen again(It's reproducible) . @jojochuang

Copy link
Contributor

@ashishkumar50 ashishkumar50 left a comment

Choose a reason for hiding this comment

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

Thanks @ChenSammi for the patch, Change LGTM +1.

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.

merging it

@jojochuang jojochuang merged commit 6cfe9cf into apache:HDDS-7593 Apr 3, 2024
@ChenSammi
Copy link
Contributor Author

Thanks @jojochuang and @ashishkumar50 for the review.

chungen0126 pushed a commit to chungen0126/ozone that referenced this pull request May 3, 2024
chungen0126 pushed a commit to chungen0126/ozone that referenced this pull request May 3, 2024
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024
… have been initialized already' (apache#6467)

(cherry picked from commit 6cfe9cf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hbase HBase on Ozone support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants