errose28 commented on code in PR #10099:
URL: https://github.com/apache/ozone/pull/10099#discussion_r3155500582
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveDeletedKeysRequest.java:
##########
@@ -52,6 +52,11 @@ public OMSnapshotMoveDeletedKeysRequest(OMRequest omRequest)
{
@Override
@DisallowedUntilLayoutVersion(FILESYSTEM_SNAPSHOT)
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
Review Comment:
This change is for this line in the PR description:
>When this was applied to OMLayoutFeatureAspect, it exposed a bug where some
existing snapshot requests had not applied the annotation to preExecute and
were checking against a default (always finalized) OMLayoutVersionManager. This
buggy fallback path has been removed and the annotations moved to their correct
spot.
`OMLayoutFeaureAspect` does not have explicit handling for
`@DisallowedUntilLayoutVersion` on `validateAndUpdateCache`. This caused it to
fall back to `OMLayoutFeaureAspect` line 73 in the original code, which creates
a default version manager with no knowledge of the apparent/metadata version.
This means the annotation actually did not block anything because the
evaluation thought OM was finalized.
I removed this fallback code path when I updated `OMLayoutFeaureAspect` and
failing tests exposed this issue. The `preExecute` override here only exists to
be annotated. In a future change it would be good to create some sort of test
that makes sure the annotations are only put on supported methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
