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