OK, let me open PRs for branch-2 and branch-2.6 too.
Istvan Toth <st...@cloudera.com.invalid> 于2024年6月13日周四 14:02写道： > > I am also concerned about this change on branch-2. > Some of our code does implement getSequenceId() , so doing this on 2.5 and > 2.6 would definitely break the existing releases, and require some changes > and new compatibility modules on the Phoenix side. > > Phoenix usually uses the KeyValue type internally, which already extends > ExtendedCell, so it's probably not a huge problem, but we'd like to check > before this is merged to branch-2. > I'd like to ask for a heads-up when we have a PR for branch-2 so that we > can check what breaks. > (We currently only support branch-2.6, so ideally we'd have a branch-2.6 PR > to check, but we can probably get branch-2 to work to check the change > without a lot of work) > > We also have a POC branch for HBase 3, so we can also check Phoenix with a > branch-3 version of this change. > > The worst case scenario for Phoenix would be having to open a different > branch for hbase 3.x support, we really want to avoid that. > As long as the change can be papered over with a compatibility module, > we're mostly fine. > > Istvan > > > > On Thu, Jun 13, 2024 at 5:45 AM 张铎(Duo Zhang) <palomino...@gmail.com> wrote: > > > In fact, I do not think CPs can really return their own Cell > > implementation which is not an ExtendedCell, we will call > > PrivateCellUtil.setSequenceId and setTimestamp at server side, if the > > Cell is not an ExtendedCell we will throw IOException... > > > > I guess the only place where it is safe to return a customized Cell is > > in filter implementation, in getNextCellHint, where we will only use > > it to seek the scanners without storing it anywhere... > > > > Reid Chan <reidchan0...@gmail.com> 于2024年6月13日周四 10:56写道： > > > > > > It may affect Phoenix, as it is CPs related, at least for branch-2. > > > > > > Let's wait and see if there are any comments from them > > > > > > > > > --- > > > > > > Best regards, > > > R.C > > > > > > > > > > > > On Tue, Jun 11, 2024 at 4:19 PM Duo Zhang <zhang...@apache.org> wrote: > > > > > > > We have several deprecated methods in Cell interface, like > > > > getSequenceId and tag related ones, which are marked as deprecated > > > > since 2.0.0 and should be removed in 3.0.0. Most of them are marked as > > > > deprecated in HBASE-19112. > > > > > > > > After investigating, I found that it is not an easy work... > > > > > > > > We have 3 levels for the Cell interface > > > > > > > > Cell -> RawCell -> ExtendedCell > > > > > > > > Where Cell is the Public API for client side usage, RawCell is for CPs > > > > where we expose the tag related APIs, and ExtendedCell is for server > > > > side usage, where we expose all the internal stuff, like sequence id. > > > > > > > > In HBASE-19550, we introduced a CellWrapper as we think maybe CPs will > > > > return a Cell which is not an ExtendedCell at server side, we need to > > > > wrap it so at server side we always get an ExtendedCell. So if we > > > > remove the tag and sequence id related methods, CellWrapper will be > > > > broken. > > > > > > > > For me, I do not think we need the CellWrapper. In general, all actual > > > > Cell implementation classes in our code base implement the > > > > ExtendedCell interface, i.e, we can simply consider all Cells are > > > > actually ExtendedCells. The only reason we introduce the Cell > > > > interface, is to hide internal stuff from client public API, does not > > > > mean we want users to implement their own Cell. > > > > > > > > So the plan is to change server side code use ExtendedCell as much as > > > > possible in HBASE-28644, a draft PR is already there for review[1]. > > > > This will be landed to both 3.x and at least branch-2(I'm not sure if > > > > it worth to also land on branch-2.6 and branch-2.5). > > > > And starting from 3.0.0, we should make clear that all Cell instances > > > > should be created via the CellBuilder related APIs, other Cell > > > > implementations are not allowed. So on 3.0.0, we should treat Cell as > > > > ExtendedCell everywhere in our code base, and also remove the > > > > CellWrapper class, then we could finally remove the deprecated methods > > > > in the Cell interface. > > > > > > > > Thoughts? > > > > > > > > Thanks. > > > > > > > > 1. https://github.com/apache/hbase/pull/5976 > > > > > > > > > -- > *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> > ------------------------------ > ------------------------------
