feat: Add sync machine button & correct network address - #29
Conversation
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| with filelock(f"machine-sync-{self.name}", timeout=SYNC_LOCK_TIMEOUT): | ||
| self.reload() | ||
| before = self.status | ||
|
|
||
| return self.sync(client) != before |
There was a problem hiding this comment.
The file lock is released when sync_exclusively returns, but self.save() remains uncommitted until the enclosing request or background job finishes. A second worker can acquire the lock during that window, reload the still-committed Pending state, and process the same transition again. Both callers may then notify the owner, while the second save may overwrite concurrent data or raise a write conflict. Keep synchronization in effect through the transaction commit or use a database-level concurrency mechanism.
Prompt To Fix With AI
This is a comment left during a code review.
Path: cargo/cargo/doctype/machine/machine.py
Line: 153-157
Comment:
**Lock Ends Before Commit**
The file lock is released when `sync_exclusively` returns, but `self.save()` remains uncommitted until the enclosing request or background job finishes. A second worker can acquire the lock during that window, reload the still-committed `Pending` state, and process the same transition again. Both callers may then notify the owner, while the second save may overwrite concurrent data or raise a write conflict. Keep synchronization in effect through the transaction commit or use a database-level concurrency mechanism.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if moved: | ||
| notify_owner(self.reference_doctype, self.reference_name) |
There was a problem hiding this comment.
If the first owner notification fails, such as when a concurrent owner edit makes sync_machines() raise a timestamp conflict, the exception is only logged while the machine transition still commits. Later manual syncs see the settled status and skip notification, and the scheduled sweep no longer selects the non-Pending machine. This can permanently leave the owning Datum Server or Object Storage Cluster out of sync with its machine, so owner reconciliation needs a retryable path instead of being limited to the original transition.
Prompt To Fix With AI
This is a comment left during a code review.
Path: cargo/cargo/doctype/machine/machine.py
Line: 146-147
Comment:
**Failed Notifications Never Retry**
If the first owner notification fails, such as when a concurrent owner edit makes `sync_machines()` raise a timestamp conflict, the exception is only logged while the machine transition still commits. Later manual syncs see the settled status and skip notification, and the scheduled sweep no longer selects the non-Pending machine. This can permanently leave the owning Datum Server or Object Storage Cluster out of sync with its machine, so owner reconciliation needs a retryable path instead of being limited to the original transition.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
1 participant
No description provided.