nick-boss-tech opened a new pull request, #5100: URL: https://github.com/apache/solr/pull/5100
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-18532 ## What happens today **A failed restore or install can leave the core pointing at the wrong index directory.** When a restore or install fails after the switch to the restored index, `RestoreCore` rolls back. The rollback deletes `index.properties`. That is correct only when the core used the default `index` directory before the restore. If the core used another index directory, the delete drops the pointer to it. After that, the core opens the default `index` directory instead. ## What this change does **On rollback, the core's previous `index.properties` is written back.** Before the switch, the change reads the file's bytes. If the switch fails, it writes those same bytes back. The file is deleted only when the core had no `index.properties` before the restore. The change is in `RestoreCore.java`, with a new test for the non-default case. No public method changes. This is a product fix. It does not address the intermittent teardown failure seen in the install tests. Behavior change: a read error other than "file not found" now stops the restore before the switch. Before, that error was ignored. ## Proof **The new test fails on the base code and passes with this change.** `TestRestoreCore.testFailedRestoreKeepsNonDefaultIndexPointer` first runs a restore that succeeds, so the pointer names a restored directory. It then runs a restore that fails. With `RestoreCore.java` reverted to the base commit and the test kept, the pointer check fails. The check expects the earlier restored directory, but the pointer is missing (null), because the old rollback deletes the file. With this change, the test passes. - `TestRestoreCore`: 4 of 4 pass at `f1e5031fc3a9624b03daf353f26c977891d7d876`, verified 2026-10-10. - `RestoreCoreOpTest`: 1 of 1 passes at the same head, verified 2026-10-10. ## A choice to check **The rollback writes back the saved file bytes, not only the directory name.** The implemented route saves the file's exact bytes and writes them back. The other route saves only the previous directory name and sets it again through the existing modify path. Both routes fix the case the test covers. The exact-bytes route was chosen here; if the name route reads better, I can switch. ## Limits - The proof runs on the in-memory test directory factory. The atomic file move used by the file-system factory is not run by these tests. - If writing the saved bytes back fails, the error stops the rest of the rollback. The writer is not reopened and the restore directory is not removed. The base code has the same shape around its delete. - If the write fails, a temporary `index.properties.<number>` file can be left in the data directory. Nothing removes it. The existing code has the same pattern. - On a factory that deletes and then renames, a failure between those two steps can leave no pointer. Only the in-memory factories use that fallback today. This is not new. - The test checks the pointer after the rollback. It does not check that the file is absent when the core had no earlier file. - The intermittent teardown failure in the install tests comes from another path. This change does not address it. Changelog: `changelog/unreleased/SOLR-18532.yml` ### AI assistance AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
