Skip to content

HDDS-8310. Recon goes down with RepeatedOmKeyInfo cannot be cast to OmKeyInfo#5043

Merged
jojochuang merged 9 commits intoapache:masterfrom
ArafatKhan2198:HDDS-8310
Jul 13, 2023
Merged

HDDS-8310. Recon goes down with RepeatedOmKeyInfo cannot be cast to OmKeyInfo#5043
jojochuang merged 9 commits intoapache:masterfrom
ArafatKhan2198:HDDS-8310

Conversation

@ArafatKhan2198
Copy link
Contributor

@ArafatKhan2198 ArafatKhan2198 commented Jul 11, 2023

What changes were proposed in this pull request?

This patch aims to address a bug in Recon that triggers an error with the following message: "org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo cannot be cast to org.apache.hadoop.ozone.om.helpers.OmKeyInfo".

The error occurs during event processing in the Recon server, specifically within the NSSummaryTaskWithFSO class at
line 91. It appears to be a class casting issue, where an object of type RepeatedOmKeyInfo is mistakenly cast as OmKeyInfo.

To resolve this bug, we recommend modifying the OMDBUpdatesHandler class responsible for creating OmdbUpdateEvents. The approach involves ensuring that the events are not created with incorrect object types, as these are considered invalid.

To implement this solution, we introduce a new utility class called OmUpdateEventValidator. This class is designed to validate OMDBUpdateEvents and can be extended for different types of validations. Its purpose is to check the expected value type for a given table against the actual value type provided in the event. If the types do not match, the validation fails, and the even is not consumed.

What is the link to the Apache JIRA

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

How was this patch tested?

The existing unit tests passed successfully, and additional unit tests were added to test the validation class.

@adoroszlai adoroszlai changed the title HDDS-8310. Recon goes down with RepeatedOmKeyInfo cannot be cast to org.apache.hadoop.ozone.om.helpers.OmKeyInfo. HDDS-8310. Recon goes down with RepeatedOmKeyInfo cannot be cast to OmKeyInfo Jul 11, 2023
@ArafatKhan2198
Copy link
Contributor Author

@jojochuang @ChenSammi @smengcl @devmadhuu Could you please take a look!

Copy link
Contributor

@devmadhuu devmadhuu left a comment

Choose a reason for hiding this comment

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

Pls check and handle comments.

Copy link
Contributor

@devmadhuu devmadhuu left a comment

Choose a reason for hiding this comment

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

Validator looks good. As discussed, With validator , we may not need changes in downstream ReconOmTask classes. You can revert and check logic earlier put.

Add positive and negative test cases for EventValidator.

@devmadhuu
Copy link
Contributor

@ArafatKhan2198 - Pls also update PR description which reflects the current change as no need to do any changes in downstream classes.

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.

Looks almost ready. Just a few more nitpicks

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 pending precommit test @devmadhuu @smengcl anything you'd like to add?

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

@ArafatKhan2198 Thanks for working over this,
We need analyze further with Recon DB available to check why oldValue type is wrong,

  • check recon DB
  • check from memory if really old value type is wrong for fileTable

Copy link
Contributor

@devmadhuu devmadhuu left a comment

Choose a reason for hiding this comment

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

@ArafatKhan2198 Thanks for working on this patch. LGTM +1

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

@ArafatKhan2198 LGTM +1

@jojochuang jojochuang merged commit 58434d2 into apache:master Jul 13, 2023
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.

6 participants

Comments