Hi Istvan, I think that your assessment is correct and your proposal makes sense to me. Let's keep it clear that this is a public interface.
For the branch-2 changes, I'm less sure. Well, I agree in principle, but I'm not sure if this can be achieved without a breaking change. I'm happy to be proven wrong. Good on you! Thanks, Nick On Fri, Jul 12, 2024 at 12:42 PM Istvan Toth <st...@cloudera.com.invalid> wrote: > HBASE-23975 was the original ticket. > > My guess is that since hbase-shaded-protocol was already set up to do the > compiling and shading, moving it there was the easiest solution. > I guess that the same logic was behind the rename: since every other class > there uses the .shaded. package, change the REST messages the same way. > > regards > Istvan > > > > > On Fri, Jul 12, 2024 at 9:48 AM 张铎(Duo Zhang) <palomino...@gmail.com> > wrote: > > > In which jira we did this moving? Are there any reasons why we did > > this in the past? > > > > Istvan Toth <st...@apache.org> 于2024年7月12日周五 03:57写道： > > > > > > Hi! > > > > > > While working on HBASE-28725, I realized that in HBase 3+ the REST > > protobuf > > > definition files have been moved to hbase-shaded-protobuf, and the > > package > > > name has also been renamed. > > > > > > While I fully agree with the move to using the thirdparty protobuf > > library > > > (in fact I'd like to backport that change to 2.x), I think that moving > > the > > > .proto files and renaming the package was not a good idea. > > > > > > The REST interface does not use the HBase patched features of the > > protobuf > > > library, and if we want to maintain any pretense that the REST protobuf > > > encoding is usable by non-java code, then we should not use it in the > > > future either. > > > > > > (If we ever decide to use the patched features for performance reasons, > > we > > > will need to define new protobuf messages for that anyway) > > > > > > Protobuf does not use the package name on the wire, so wire > compatibility > > > is not an issue. > > > > > > In the unlikely case that someone has implemented an independent REST > > > client that uses protobuf encoding, this will also ensure compatibility > > > with the 3.0+ .protoc definitions. > > > > > > My proposal is: > > > > > > HBASE-28726 <https://issues.apache.org/jira/browse/HBASE-28726> Revert > > REST > > > protobuf package to org.apache.hadoop.hbase.shaded.rest > > > *This applies only to branch-3+:* > > > 1. Move the REST .proto files and compiling back to the hbase-rest > module > > > (but use the same protoc compiler that we use now) > > > 2. Revert the package name of the protobuf messages to the original > > > 3. No other changes, we still use the thirdparty protobuf library. > > > > > > The other issue is that on HBase 2.x the REST client still requires > > > unshaded protobuf 2.5.0 which brings back all the protobuf library > > > conflicts that were fixed in 3.0 and by hbase-shaded-client. To fix > this, > > > my proposal is: > > > > > > HBASE-28725 <https://issues.apache.org/jira/browse/HBASE-28725> Use > > > thirdparty protobuf for REST interface in HBase 2.x > > > *This applies only to branch-2.x:* > > > 1. Backport the code changes that use the thirdparty protobuf library > for > > > REST to branch-2.x > > > > > > With these two changes, the REST code would be almost identical on > every > > > branch, easing maintenance. > > > > > > What do you think ? > > > > > > Istvan > > > > > -- > *István Tóth* | Sr. Staff Software Engineer > *Email*: st...@cloudera.com > cloudera.com <https://www.cloudera.com> > [image: Cloudera] <https://www.cloudera.com/> > [image: Cloudera on Twitter] <https://twitter.com/cloudera> [image: > Cloudera on Facebook] <https://www.facebook.com/cloudera> [image: Cloudera > on LinkedIn] <https://www.linkedin.com/company/cloudera> > ------------------------------ > ------------------------------ >
