Reminder. The hadoop community is currently upgrading the protobuf dependency to 3.x, and it will also be a shaded and relocated version, so I think it is also the time for us to upgrading the protobuf version for CP now.
Let me open an issue for this. We can discuss there on how to do the upgrading. Thanks. Stack <[email protected]> 于2019年8月28日周三 上午1:41写道： > On Tue, Aug 27, 2019 at 8:51 AM Stack <[email protected]> wrote: > > > On Tue, Aug 27, 2019 at 2:11 AM 张铎(Duo Zhang) <[email protected]> > > wrote: > > > >> I think we could let the CPs just use our shaded version of protobuf? > And > >> we can make a guarantee that we will not upgrade the minor version of > >> protobuf unless we have a new major release. > >> > >> > > We could make a call that CPEPs move to use our shaded protobuf. > > > > This would be us relaxing our dictum that CPEPs NOT rely on hbase > internals. > > (Should pb protos get an InterfaceAudience?) > > > > > >> And for the compatibility, as said above, we already breaks lots of CP > for > >> a new major release, which means the users already need to reimplement > the > >> CPs, so it is not a big deal to also upgrade the protobuf to our shaded > >> version. > > > > > > This was not the case for CPEPs. CPEPs define their own APIs using pb ato > get in the frontdoor and then run their own code serverside. They were not > explicitly dependent on CP APIs. Testing had it that at least for those > tested, CPEPs written for hbase1 continued to work against hbase2. > > But that was then. We can make a call that CPEPs need to be changed if > moving to hbase3. > > > > > Maybe we should provide a tool to help them relocating the imports > >> in the generated file? > > > > > There is a nice (errorprone? checkstyle?) plugin in our build currently > that warns if a non-shaded reference to protobuf. We could write up a > recipe for CPEPs showing how they might do the same. > > S > > > > > > And protobuf 2.5.x and 3.x are wire compatible, so > > > > it is still possible to keep compatible with old CP client which still > uses > >> 2.5.0 as you can share the same protobuf defination file. > >> > > > > > > Thanks. > >> > >> Stack <[email protected]> 于2019年8月27日周二 下午12:51写道： > >> > >> > On Mon, Aug 26, 2019 at 9:28 PM Duo Zhang <[email protected]> > wrote: > >> > > >> > > I think this is a problem for the 3.0.0 release. When upgrading to > >> 2.0.0, > >> > > we shaded the protobuf and upgrade the version to 3.x. But for > >> > coprocessor, > >> > > the protobuf version is still 2.5.0, and we made a hbase-protocol > >> module > >> > > for it. > >> > > > >> > > >> > > >> > Yes. This was intentional. Didn't want to break existing endpoint > >> > coprocessors going from branch-1 to branch-2 as part of the shade > >> protobufs > >> > project (HBASE-15638). > >> > > >> > > >> > > >> > So we still keep this solution when releasing 3.0.0? Which means the > >> > > protobuf version for coprocessor will always be 2.5.0? Seems not a > >> good > >> > > solution. > >> > > > >> > And since we are allowed to make breaking changes on the CP hooks > during > >> > > major version, I think letting the CPs use a new version of protobuf > >> is > >> > > also fine? > >> > > > >> > > Suggestions? > >> > > > >> > > > >> > > >> > The doc attached to HBASE-15638 had some thoughts on this ( > >> > > >> > > >> > https://docs.google.com/document/d/1H4NgLXQ9Y9KejwobddCqaVMEDCGbyDcXtdF5iAfDIEk/edit#heading=h.yh3l2y26xbvk > >> > ) > >> > and HBASE-16761 <https://issues.apache.org/jira/browse/HBASE-16761> > was > >> > filed for moving CPEPs to pb3. > >> > > >> > + PB Service is the mechanism CPEPs are built on. Our classpath will > >> carry > >> > PB2.5 until Hadoop moves off it so we won't be able to expose > >> > com.google.protobuf.* in our API if its not pb2.5. Expose a shaded > >> > c.g.protobuf? But at a different shade point than that of the pb we > use > >> > internally so we can continue to freely upgrade the pb we use > internally > >> > w/o breaking CPEPs? Or try to hide the implementation? And so on.... > >> > > >> > Thanks for bringing this up. > >> > > >> > S > >> > > >> > > >> > > >> > > >> > > Thanks. > >> > > > >> > > >> > > >
