Thanks Istvan! I agree with your points. It’s really been a while we are talking about releasing 5.2.0 and yet due to bandwidth issues, unable to do so.
I agree to getting these fixes out, cut 5.2 and start the release work and in the meantime I also need to prepare 5.1 backport. For 5.3, let's plan HBase 3.0 support and JSON as major changes. On Mon, Jan 22, 2024 at 9:17 PM Istvan Toth <st...@apache.org> wrote: > Hi! > > In my opinion cutting 5.2 now only makes sense IF we DO NOT plan to release > the outstanding big features (like JSON) in 5.2. , otherwise it's just > extra work to maintain more branches. > > Having said that, releasing a 5.2 and 5.1.4 with the data integrity fixes > real soon, and then releasing 5.3 in a few months with JSON, and any other > outstanding big features > that are close to being finished (and HBase 3.0 support, if it's ready by > then) would not be a bad idea. > > On the CLDR side the only outstanding big feature which could impact > Viraj's integrity work is HBase 3.0 support, and even that is only because > it may require some larger refactors of existing code, not because it would > change the actual behaviour or algorithms. > > Phoenix used to have several minor releases per year, the current state of > extreme longevity of 5.1 and several big new features being added to it > (like uncovered indexes) is not ideal. > Releasing 5.2 and 5.3 relatively close together could be a return to a > quicker cadence for minor releases, which could also help with the public > image and adoption of Phoenix. > > We were talking about releasing 5.2 at least a year ago, and I have started > working on that then, but then emergencies have come up at $dayjob, and I > could not see that through. > (So I am in part responsible for the lack of minor releases) > > regards > Istvan > > > On Mon, Jan 22, 2024 at 11:35 PM Viraj Jasani <vjas...@apache.org> wrote: > > > Sorry for the late reply. > > > > > Do you think cutting 5.2.0 now makes sense? > > > > No problem with that. I can cut 5.2 branch by the end of this week or at > > the start of next week. > > > > If there is any very big change or feature ready for merge to master > branch > > with PR approvals already in place, please do let me know so that I can > > help collaborate on how best we can get it merged without impacting 5.2 > > release if required. My main motivation was for any big change to go > > through newly introduced tests so that we know that anything additional > is > > not broken, and also to prioritize for upcoming 5.2.0 and 5.1.4 releases. > > Moreover, there are several PRs getting merged on the master branch, we > can > > continue that as long as they are not very big changes, which might > require > > significant time to understand any correlation with data integrity > issues. > > > > The PR is also ready for review with some additional cases fixed last > week: > > https://github.com/apache/phoenix/pull/1736 > > > > Depending on the review bandwidth, I am hopeful we should be good to land > > them sooner. > > > > > > On Wed, Jan 17, 2024 at 11:31 AM Rushabh Shah > > <rushabh.s...@salesforce.com.invalid> wrote: > > > > > Thank you Viraj for initiating this thread. > > > > > > > Given the critical nature of these issues, I would like to propose > that > > > we > > > treat this as a high priority for the upcoming 5.2.0 release, and not > > > include any other feature or big change to master branch until we merge > > > this. > > > > > > Do you think cutting 5.2.0 now makes sense? This will enable other > > > developers to merge features into master branch (5.3.0) and you can > take > > > some more time to make sure we cover all the corner cases for the data > > > integrity issues that you uncovered. > > > > > > > > > On Fri, Jan 5, 2024 at 6:38 PM Viraj Jasani <vjas...@apache.org> > wrote: > > > > > > > Sounds good, thanks Rajeshbabu. I will try to get the first PR out > next > > > > week and while reviews happen in parallel, will try to get 5.1 PR > soon. > > > > > > > > > > > > On Wed, Jan 3, 2024 at 8:49 PM rajeshb...@apache.org < > > > > chrajeshbab...@gmail.com> wrote: > > > > > > > > > Hi Viraj, > > > > > > > > > > Would be better to include the changes in 5.1.4 as in any way it > > will > > > > take > > > > > at least 3-4 days to complete the omid release. > > > > > > > > > > Thanks, > > > > > Rajeshbabu. > > > > > > > > > > > > > > > On Thu, Jan 4, 2024 at 5:06 AM Viraj Jasani <vjas...@apache.org> > > > wrote: > > > > > > > > > > > Thank you Kadir and Geoffrey for your replies!! > > > > > > > > > > > > > How does this affect 5.1.4, which is also listed as a Fix > Version > > > for > > > > > > > PHOENIX-7106? > > > > > > > > > > > > Yes, it also needs to be ported to 5.1. Once the master PR is up > > for > > > > > final > > > > > > review, I would start working on the backport PR. > > > > > > We just need some more additional testing to ensure old client > > (e.g. > > > > > 5.1.3) > > > > > > is compatible with the new server with the changes. > > > > > > > > > > > > Hence, yes it is now a blocker for upcoming 5.1.4 as well since > > 5.1.4 > > > > RC > > > > > > preparation is still pending (while Omid release is in progress). > > > > > > Otherwise if 5.1.4 was ready for release, I would have proposed > > > > immediate > > > > > > 5.1.5 release to include the changes proposed with PHOENIX-7106. > > > > > > > > > > > > > > > > > > On Wed, Jan 3, 2024 at 3:08 PM Geoffrey Jacoby < > gjac...@apache.org > > > > > > > > wrote: > > > > > > > > > > > > > I agree that data integrity issues are a higher priority than > > > feature > > > > > > > development, so I also support the decision. The fact that > > several > > > of > > > > > the > > > > > > > major remaining 5.2 features are currently being developed in > > > > > > long-running > > > > > > > feature branches also helps, as work can continue there at the > > cost > > > > of > > > > > a > > > > > > > rebase later. > > > > > > > > > > > > > > How does this affect 5.1.4, which is also listed as a Fix > Version > > > for > > > > > > > PHOENIX-7106? From the bug description it also sounds like > 5.1.3 > > > and > > > > > the > > > > > > > forthcoming .4 are affected, since we have server-side paging > in > > > 5.1. > > > > > > (Feel > > > > > > > free to move that to a separate thread if you feel it should > be a > > > > > > separate > > > > > > > discussion.) Should this be a blocker for releasing 5.1.4? > > > > > > > > > > > > > > Geoffrey > > > > > > > > > > > > > > > > > > > > > On Wed, Jan 3, 2024 at 5:06 PM Kadir Ozdemir < > > > > > > > ka...@gsuite.cloud.apache.org> > > > > > > > wrote: > > > > > > > > > > > > > > > Being a database, Phoenix has to make sure that the data > stays > > on > > > > > disk > > > > > > > > intact and its queries return correct data. In this case, > > Phoenix > > > > > fails > > > > > > > to > > > > > > > > return correct data for some queries if their scans > experience > > > > region > > > > > > > > movement. Now that we know these data integrity issues and > how > > to > > > > > > > reproduce > > > > > > > > them, fixing them should be our first priority. So, I fully > > > support > > > > > > this > > > > > > > > proposal. > > > > > > > > > > > > > > > > On Wed, Jan 3, 2024 at 10:58 PM Viraj Jasani < > > vjas...@apache.org > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > > > I would like to bring PHOENIX-7106 > > > > > > > > > < > > > > > > > > > > https://urldefense.com/v3/__https://issues.apache.org/jira/browse/PHOENIX-7106__;!!DCbAVzZNrAf4!FZG5sv55IC1NqItQLY7GKWgUG2Do0gSta01gOiSdd36Dx3XHGtQx4M3c9visVXIt9DctPQzS-orob9vhzrCfVA$ > > > > > to everyone's > > > > > > > > > attention here and brief about the data integrity issues > that > > > we > > > > > have > > > > > > > in > > > > > > > > > various coprocessors. Majority of the issues are related to > > the > > > > > fact > > > > > > > that > > > > > > > > > we do not return valid rowkey for certain queries. If any > > > region > > > > > > moves > > > > > > > in > > > > > > > > > the middle of the scan, the HBase client relies on the last > > > > > returned > > > > > > > > rowkey > > > > > > > > > and accordingly changes the scan boundaries while the > scanner > > > is > > > > > > > getting > > > > > > > > > reset to continue the scan operation. If the region does > not > > > > move, > > > > > > scan > > > > > > > > is > > > > > > > > > not expected to return invalid data, however if the region > > > moves > > > > in > > > > > > the > > > > > > > > > middle of ongoing scan operation, scan would return > > > > > invalid/incorrect > > > > > > > > data > > > > > > > > > causing data integrity issues. > > > > > > > > > > > > > > > > > > Given the critical nature of these issues, I would like to > > > > propose > > > > > > that > > > > > > > > we > > > > > > > > > treat this as a high priority for the upcoming 5.2.0 > release, > > > and > > > > > not > > > > > > > > > include any other feature or big change to master branch > > until > > > we > > > > > > merge > > > > > > > > > this. The PR is still not ready as additional changes are > > still > > > > in > > > > > my > > > > > > > > > local, requiring rebase with the current master. > > > > > > > > > > > > > > > > > > I would get back to this discuss thread as soon as the PR > and > > > the > > > > > doc > > > > > > > are > > > > > > > > > updated with the latest findings so far. The changes > include > > > many > > > > > of > > > > > > > our > > > > > > > > > coproc scanner implementations and hence it would require > > > > > significant > > > > > > > > > review as well. > > > > > > > > > It would be great if we can hold on to merging any feature > or > > > big > > > > > > > change > > > > > > > > to > > > > > > > > > master branch until this gets in so as to not complicate > > > > > > > > merging/rebasing. > > > > > > > > > Once this is merged to the master branch, I would like to > cut > > > 5.2 > > > > > > > branch > > > > > > > > > from master and we can move forward with 5.2.0 release. > > > > > > > > > > > > > > > > > > Please let me know if this looks good or if you have any > > other > > > > high > > > > > > > > > priority work for 5.2.0. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
