smengcl commented on PR #4567: URL: https://github.com/apache/ozone/pull/4567#issuecomment-1509225002
> I also think some of the snapshot user code should be modified to support try-with-resources. For example, this code: > > https://github.com/apache/ozone/blob/ddfc4b738d6d48ef37f3af41a7f743247933e30a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java#L83-L91 > > does the checkForSnapshot() call and then passes a reference to the omSnapshot into the double buffer thread. > > I think those sorts of calls should be modified just to pass the snapshot key into the double buffer thread and have it get the snapshot from the cache, (with try-with-resources.) > > The way it is implemented now, the reference is passed between two threads, and try-with-resources requires it all happen in the same thread. Back when @aswinshakil was implementing this in #4486 I [asked the same question](https://github.com/apache/ozone/pull/4486#discussion_r1151192924) of why didn't we just pass the snapshot table key to the `Response` and get the instance there. The answer was also encapsulation. `Response` only gets the `OMMetadataManager` instance (active or snapshot DB) it is operating on. Though I think passing in the extra `omSnapshotManager` alone is fine. -- 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]
