Yea, that makes sense and was the one reason I could think of as well. My concern with not backporting is that 3.x has been in development for many years, and all of our users are on 2.x (or even 1.x). Duo is making progress on 3.x now, but it's not clear when it will be released or, further, when people will upgrade. So IMO features that land just in master/branch-3 may not see any users for a long time. All of the active development for users gets backported to 2.x, and unnecessarily keeping big diffs between 3.x and 2.x just increases the backport burden (merge conflicts). There are some good reasons for certain major feature changes to have such a diff, but not sure it warrants it for this.
I am guessing the changes in your PR which break v1 are related to either removal of certain enum options or what the default values are. I suggest that we _do_ backport this to branch-2, but in that backport we add back whatever enum options are necessary to keep support for both v1 and v2. We can say that v1 is dropped in 3.x. If necessary, we can add a config in branch-2 to specify which version of async-profiler to use (like if we need to determine which default values to use). Thoughts? On Thu, Jan 18, 2024 at 8:42 AM Lars Francke <lars.fran...@gmail.com> wrote: > Hi Bryan, > > good question. We can probably keep supporting v1 if we wanted to but > I'm wondering if it's worth the effort (personally I think not). > I do agree that it should not be backported! > > And I have wondered the same. If you look at the history of the issue > I kinda proposed forwarding parameters as well but then thought that > it would mean giving someone "access" to a shell with HBase privileges > and we'd need to deal with sanitizing inputs and making sure people do > nothing malicious. > That was a task that I wasn't prepared to tackle so I opted to keep > the current route and just update it. > > I hope that makes sense? > > Cheers, > Lars > > On Thu, Jan 18, 2024 at 2:16 PM Bryan Beaudreault > <bbeaudrea...@apache.org> wrote: > > > > Is it necessary to break support for v1? > > > > It would be fine to merge this to master and branch-3. But I think this > > change falls into the "Dependency Compatibility" portion of our > > compatibility guidelines[1]. If so, then we should not backport to > branch-2 > > or 2.x. > > > > Generally, I have previously wondered what value the Event and Output > enums > > provide. It requires us to keep the enums updated with async-profiler > > options, when we could just directly send the user's input to > > async-profiler. If they specify a bad option, maybe we can show them the > > error message from async-profiler. This approach might allow for any > > async-profiler version. > > > > [1] https://hbase.apache.org/book.html#hbase.versioning.post10 > > > > On Thu, Jan 18, 2024 at 3:45 AM Lars Francke <lars.fran...@gmail.com> > wrote: > > > > > Hi, > > > > > > I just wanted to give a heads-up on an issue I created (thanks for > > > everyone's help)[1] > > > > > > This would be a breaking change as it removes support for > > > async-profiler v1 but in exchange it does add support for various v2 > > > features and in general "unbreaks" things that were broken. > > > > > > I already have an approved PR[2] but wanted to give the chance to > > > comment before I merge that sometimes in the next few days or weeks > > > > > > Cheers, > > > Lars > > > > > > > > > [1] <https://issues.apache.org/jira/browse/HBASE-28242> > > > [2] <https://github.com/apache/hbase/pull/5566> > > > >
