Multi-node correctness fixes for deployments using a shared external JMS broker#15
Conversation
…uing a Rebuild (#12 & #13) Issue: - Reimport queued an Operation.Rebuild that, on auto-archive destinations, fans a Rebuild -> Archive op through the shared JMS queue to both nodes. - XNAT's non-atomic archive gate lets both run, so they collide: "Session already exists with matching files" or "FileNotFoundException: <session>.xml". Solution: - Pass action=commit and conditional AA=true to DicomZipImporter so it builds and archives inline on the importer thread - AutoArchive (AA) is conditional as not to override destination project archive settings.
- Batch completion: replace the per-JVM counter with a shared BatchTransferProgress row incremented under SELECT ... FOR UPDATE, so the terminal event fires once when items span nodes.
- Harden completion counter: >= total with detect-and-remove in one locked transaction, so JMS redelivery can't strand or double-fire the terminal event - Tolerate the cross-node register insert race (catch DataIntegrityViolationException) - Drop unused currentPercent and its per-item DB reads (the percent was never surfaced) - Gate inline auto-archive on prearchive code >= AutoArchive, matching XNAT's FinishImageUpload - Surface the root-cause message to users and workflow details instead of the generic wrapper - Use Lombok accessors on the entity and Completion value type
kathrynalpert
left a comment
kathrynalpert
left a comment
There was a problem hiding this comment.
Overall, if we already have workflows for clone/reimport/share actions, why also use event tracking? Should we use the workflow table directly like BulkLaunch: NrgXnat/xnat-web@218c981?
Separately - reminder to bump version numbers/CHANGELOGs if appropriate.
| params.put("rename", "true"); | ||
| params.put("project", destinationProjectData.getId()); | ||
| params.put("action", "commit"); | ||
| // AA drives DicomZipImporter.isAutoArchive(), which ignores the project's prearchive code, so |
There was a problem hiding this comment.
Is this true? Seems like a bug/oversight if so.
There was a problem hiding this comment.
This is true - verified on XNAT 1.9.3.5. I'm not sure if it's a bug or just a way to explicitly override the project setting. Although this setting to enable it doesn't exist today, I can see a user wanting Batch Transfer to always archive a reimport on transfer - as is done with cloning and sharing.
Overall, I agree that pure workflow tracking would be better. One reason it was not used here is to enable monitoring the status of transfer items before they start the import process. Also, I wanted to track overall batch completion/failure. I see that there are patterns to do this with workflows ala batch launch, but since it involves larger change, I'm inclined to add that to ROADMAP and address it in a separate PR. |
…acking update to ROADMAP.
kelseym
force-pushed
the
double-archive-issue
branch
from
b8536c8 to
750295c
Compare
2 participants
Multi-node correctness fixes for deployments using a shared external JMS broker (#12, #13, #14). Single-node deployments are unaffected.
Rebuild → Archivechain ran on both nodes and raced XNAT's non-atomic archive gate (Session already exists with matching files/ missing.xml). Reimport now archives inline (action=commit), so nothing is queued for a second node;AA=trueis set only for auto-archive destinations.BatchTransferProgressrow incremented under aSELECT … FOR UPDATElock, so exactly one node fires the event once.