Hi Nick, Thanks for looking into this.
Actually, the branch-2 changes have already been committed. The REST interface doesn't use any Protobuf 2 specific features, so there shouldn't be any wire incompatibilities. I forgot about the branch-3 changes, but I will try to get a PR up in the next few weeks. (After I'm done with the Hadoop packaging changes) Istvan On Wed, Sep 25, 2024 at 2:33 PM Nick Dimiduk <ndimi...@apache.org> wrote: > 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> > > ------------------------------ > > ------------------------------ > > > -- *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> ------------------------------ ------------------------------
