HDDS-4303. Ensure LayoutVersion is unique and monotonicaly increasing in LayoutFeature enums

[ivandika3]

What changes were proposed in this pull request?

This change introduces a simple validation logic in AbstractLayoutVersionManager to check that LayoutFeature's layoutVersion is strictly ascending. By right "monotonically increasing" means it can have duplicates (i.e. not "unique"), but since it also specifies "unique", I would assume that means strictly increasing.

FYI: This is my first PR taken from the Newbie Ozone jiras.

What is the link to the Apache JIRA

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

How was this patch tested?

Unit Tests

• Decreasing LayoutVersions (Must fail)
• Non-unique increase LayoutVersions (Must fail)

[ivandika3]

@errose28 @fapifta @symious Could you take a look? Please let me know whether the interpretation of the issue is accurate.

[errose28]

Hi @ivanandika98 welcome to Ozone! I think the intent for this change was just a unit test to verify this for each layout feature enum in the LayoutFeature subclasses. So similar to what you have in validateLayoutFeatureVersions except we can just run that in a test and fail there without needing to check it at runtime.

[ivandika3]

@errose28 Thank you for the clarification. Sure, I will remove the validation logic and make it part of the unit tests for the LayoutFeature's subclasses.

[ivandika3]

Hi @errose28, I think the unit tests have been implemented in TestHDDSLayoutVersionManager.testHDDSLayoutFeaturesHaveIncreasingLayoutVersion and TestOMVersionManager.testOMLayoutFeaturesHaveIncreasingLayoutVersion respectively.
In that case, I think we can close this PR and the corresponding Jira ticket.

[symious]

Seems TestOMVersionManager.testOMLayoutFeaturesHaveIncreasingLayoutVersion is not checking the ordinal number.

[ivandika3]

@symious thanks for the input. I would say OMLayoutFeature.values() and HDDSLayoutFeature.values() will generate an ordinal-indexed array (from the lowest to highest ordinal), so it is indirectly accessing the ordinals of the enums.

Although it is odd that the OMLayoutFeature LayoutVersion test only assert that the LayoutVersion is increasing, while HDDSLayoutFeature LayoutVersion test additionally asserts that the first version must be 0 and only can be incremented by 1. Should we standardize these two tests?

[errose28]

Although it is odd that the OMLayoutFeature LayoutVersion test only assert that the LayoutVersion is increasing, while HDDSLayoutFeature LayoutVersion test additionally asserts that the first version must be 0 and only can be incremented by 1. Should we standardize these two tests?

Yeah let's make testOMLayoutFeaturesHaveIncreasingLayoutVersion test that the layout version is increasing by one for each enum entry. It looks like #2160 took care of most of what is described in this jira and the task was never updated, so with this minor change we can probably complete this task.

[ivandika3]

@errose28 I have standardized the layout version test on OM. Could you take another look?

[errose28]

Thanks @ivanandika98 for the test improvement. I retriggered CI and can merge when it is green.

[ivandika3]

@symious @errose28 Could you help me merge it?

[symious]

@errose28 Thanks for the review. @ivanandika98 Thank you for the contribution.

[adoroszlai]

@ivanandika98 please let us know your Apache Jira user ID so we can assign HDDS-4303 (and future issues).

[ivandika3]

Hi @adoroszlai, I'm new to Ozone and I don't currently have an Apache Jira account. Could you advise me on this?

[adoroszlai]

@ivanandika98 To request an Apache Jira account, please send an email to [email protected] with:

• preferred username (N.B. hyphens not allowed)
• alternate username (in case the preferred one is already in use)
• display name, if it is different from the username

[ivandika3]

@adoroszlai Thank you for the help. I have sent the email to [email protected].