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> ------------------------------ ------------------------------
