idantepper opened a new pull request, #4728: URL: https://github.com/apache/solr/pull/4728
https://issues.apache.org/jira/browse/SOLR-18344 # Description The `backup` key in the `/replication?command=details` response is only populated once a backup has *finished*. While one runs there is no sign of it, and the key still holds the **previous** backup's payload — including `"status": "success"`. So a client that issues `command=backup` and then polls `command=details` reads the stale `success` of an earlier backup as the result of the one it just started, and concludes the new backup is already done. With no prior backup the key is simply absent, so a poller cannot distinguish "running" from "never started" either. # Solution The plumbing already existed and was only used once. `SnapShooter.createSnapAsync` takes a `Consumer<NamedList<?>>` that `ReplicationHandler` wires to its volatile `snapShootDetails` field, which `getReplicationDetails` already publishes under the `backup` key — it was just invoked a single time, at the very end of the snapshot. That consumer is now kept in a `volatile progressListener` field and emitted through as the copy loop advances, adding two in-progress statuses ahead of the existing terminal ones: | `status` | When | Extra keys | |---|---|---| | `waiting for commit` | Published **synchronously**, before the worker thread starts | none | | `running` | After each `backupRepo.copyFileFrom` | `fileCount`, `finishedFileCount` | | `success` / exception | Unchanged | unchanged | The synchronous first publish is the part that fixes the stale-`success` read. It carries no file counts because the index commit — and with it the list of files to copy — has not been resolved yet. All in-progress statuses carry `startTime`, `directoryName`, and `snapshotName`; a null `snapshotName` is omitted rather than reported, matching how `CoreSnapshotResponse` renders a completed snapshot via `putIfNotNull`. The change is purely additive to the response, involves no API change, and **`ReplicationHandler` is untouched** — which keeps this to a single main-source file. # Tests `TestSnapshotCoreBackup#testBackupReportsProgressWhileRunning` collects every status the handler would publish and asserts on the whole sequence, rather than racing a live backup by polling — so it is deterministic. It asserts that: - the first report is `waiting for commit`, carrying neither `fileCount` nor `finishedFileCount`; - `running` reports follow, with a non-decreasing `finishedFileCount`; - every in-progress report identifies its own snapshot (`startTime`, `snapshotName`, `directoryName`); - the last `running` report accounts for every file in the completed backup; - the resulting backup still passes `simpleBackupCheck`. I verified the test fails without the fix — with the three progress emissions removed it fails on `Backup was never reported as running`. `./gradlew tidy updateLicenses check -x test` was run. Two tasks fail in my local environment for reasons unrelated to this change, both of which reproduce on unmodified `main`: - `:rat` — my checkout is a **git worktree**, where `.git` is a file rather than a directory. `rat-sources.gradle` treats that as "not a git repository", so the git-index lookup returns `null` and the `exclude "dev-docs"` / `exclude "**/AGENTS.md"` / `exclude "**/.*"` rules in that same branch never apply; rat then scans the whole tree and flags upstream files such as `dev-docs/*.adoc`, `.github/*` and `AGENTS.md`. - `:solr-ref-guide:buildLocalAntoraSite` — my repository path contains a space, which the `npx` invocation does not quote (`Error: Cannot find module '/Users/idantepper/Desktop/Developing'`). All other checks pass, including `spotlessCheck`, `ecjLint`, `forbiddenApis`, `validateLogCalls`, `validateSourcePatterns` and `javadoc`. # AI assistance disclosure This change was developed with the assistance of Claude (Anthropic), following the guidance in `dev-docs/how-to-contribute.adoc`. The commit carries a `Co-Authored-By` trailer. I have reviewed the diff and the test in full, confirmed the test fails without the fix, and I take responsibility for the contribution. # Checklist - [x] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) 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. - [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. - [x] I have developed this patch against the `main` branch. - [x] I have run `./gradlew check`. - [x] I have added tests for my changes. - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide) — not applicable; the ref guide describes `command=details` in one line and does not document the shape of its response. - [x] I have added a [changelog entry](https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc) for my change. -- 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]
