smengcl commented on pull request #2814: URL: https://github.com/apache/ozone/pull/2814#issuecomment-963509230
> Hi @smengcl, thank you for working on this forward. > > Please help me understand what is the aim here, as I am not sure why are we doing the getBucket call this way. In my head, the algorithm is simply this: > > * get the volume from the objectStore > * if volume exists get the bucket from the volume > * if volume does not exist, and if we should create it create it > * if the bucket does not exist create it > > This is better reflected by #2815, the one @dombizita created, based on our earlier discussions with her. > > I would like to understand why we are using the ObjectStore's client proxy to directly do OzoneManager protocol calls, when we can get the volume, and from the volume we can get the bucket later on, is there a good reason to turn to the underlying API I don't see, or we can even get rid of using the OzoneManager protocol proxy directly? > > On the other hand, I pretty much agree that the BucketManager#getBucketInfo(volName, bucketName) method should throw a VOLUME_NOT_FOUND in case the volume does not exist, that is a much clearer cause in this case. Thanks @fapifta for the comment. Thanks @dombizita for #2815 . > This is better reflected by #2815, the one @dombizita created, based on our earlier discussions with her. Yes #2815 is fine and it is better for compatibility (concern for compatibility is why I posted my PR #2814 in draft). #2814 and #2815 are just two different approaches to the same problem. > I would like to understand why we are using the ObjectStore's client proxy to directly do OzoneManager protocol calls The reason I used a single `proxy.getBucketDetails` call when writing OFS was to reduce unnecessary round trip between the client and OM (`getVolume` then `getBucket` adds an extra round trip). We had some discussion about the round trip concerns in the original OFS PR and offline. I am totally fine with #2815 if we are okay on taking the potential performance impact hit for now (not sure if adding an extra round trip would actually add meaningful perf impact in the first place, happy to talk about it briefly), to block other work and improve on this later. -- 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]
