smengcl edited a comment 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 can be seen as 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). There were some discussion about the extra round trip offline and IIRC in the original OFS PR as well. I am totally fine with #2815 in order to unblock other work, and improve on this later. Not sure if adding an extra round trip would really add meaningful perf impact in the first place, happy to discuss about that. -- 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]
