Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16924: Refactor requestApplyUpdate step in action=RESTORE #1965

Merged
merged 4 commits into from
Oct 6, 2023

Conversation

julia-maimone
Copy link
Contributor

https://issues.apache.org/jira/browse/SOLR-16924

Description

Refactor action=RESTORE to transition UpdateLog to ACTIVE after each replica is restored in RestoreCore call, instead of making calls to RequestApplyUpdate (which effectively transitions UpdateLog to ACTIVE state) on each newly restored replica.
The purpose of this is to simplify and clarify steps of restore. The RequestApplyUpdate op not only buffers updates, it also transitions state (the only part necessary post restoring replica).

Solution

Remove calls to RequestApplyUpdate operation on each newly restored replica, and instead complete transition of UpdateLog to ACTIVE state in each RestoreCore op after data is restored to replica.

Tests

Have run all existing unit tests in solr core module.
This is a refactor, existing tests for Backup/Restore are sufficient.

Checklist

Please review the following and check all that apply:

  • [ X] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • [ X] I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • [ X] I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

Copy link
Contributor

@dsmiley dsmiley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed CHANGES.txt:

SOLR-16924: RESTORECORE now sets the UpdateLog to ACTIVE state instead of requiring a separate REQUESTAPPLYUPDATES call in Collection restore. (Julia Lamoine, David Smiley)

A backwards-compatibility issue occurred to me. If during a Solr upgrade, the node issuing this command (the Overseer) is upgraded before the nodes it talks to, then it won't issue REQUESTAPPLYUPDATES. I'll bring this up on the dev list. I think this is fine in a major version upgrade but unsure if it's significant enough in a minor one. It would probably be a transient issue; the restore would fail and ultimately would need to be retried.

@dsmiley
Copy link
Contributor

dsmiley commented Oct 6, 2023

I'll be pushing this to main with a CHANGES.txt and upgrade note for getting to 10. In 9x I will omit the RestoreCmd part but will add a comment about it disappearing in 10.

…d of requiring a separate

  REQUESTAPPLYUPDATES call in Collection restore. The latter will still happen in 9.x for
  backwards-compatibility.  (Julia Lamoine, David Smiley)
@dsmiley
Copy link
Contributor

dsmiley commented Oct 6, 2023

Going to merge soon. 9x port will exclude the RestoreCmd part so we retain backwards-compatibility for in-place upgrades within the 9x line.

BTW I'm so glad to see Crave working so well -- the GitHub Action in entirety took 8m which is amazing. There is still room for improvement.

@dsmiley dsmiley merged commit 8740b07 into apache:main Oct 6, 2023
2 checks passed
dsmiley added a commit that referenced this pull request Oct 10, 2023
…ESTAPPLYUPDATES (#1965)

RESTORECORE now sets the UpdateLog to ACTIVE state instead of requiring a separate REQUESTAPPLYUPDATES call in Collection restore. The latter will still happen in 9.x for backwards-compatibility.

---------

Co-authored-by: Julia Maimone <julia.maimone@salesforce.com>
Co-authored-by: David Smiley <dsmiley@salesforce.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants