Skip to content

Fix/settings persistence on shutdown#582

Open
RaoufGhrissi wants to merge 1 commit intoActivityWatch:masterfrom
odoo:fix/settings-persistence-on-shutdown
Open

Fix/settings persistence on shutdown#582
RaoufGhrissi wants to merge 1 commit intoActivityWatch:masterfrom
odoo:fix/settings-persistence-on-shutdown

Conversation

@RaoufGhrissi
Copy link
Copy Markdown

@RaoufGhrissi RaoufGhrissi commented Apr 3, 2026

feat(datastore): ensure settings changes are persisted immediately
Problem:
Settings changes (UI preferences, startOfWeek) were being lost on server shutdown because they were waiting for a 15-second background commit timer.

Solution:
Ensure that SetKeyValue and DeleteKeyValue commands set self.commit = true in the datastore worker, forcing immediate persistence.

Steps to Reproduce:

  1. Start aw-server-rust.
  2. Change a setting (e.g. startOfWeek).
  3. Immediately kill the server: pkill -TERM aw-server.
  4. Restart and check the setting.

Source:
aw-datastore/src/worker.rs

Problem:
Settings changes (UI preferences, startOfWeek) were being lost on server shutdown because they were waiting for a 15-second background commit timer.

Solution:
Ensure that SetKeyValue and DeleteKeyValue commands set self.commit = true in the datastore worker, forcing immediate persistence.

Steps to Reproduce:
1. Start aw-server-rust.
2. Change a setting (e.g. startOfWeek).
3. Immediately kill the server: pkill -TERM aw-server.
4. Restart and check the setting.

Source:
aw-datastore/src/worker.rs
@RaoufGhrissi RaoufGhrissi force-pushed the fix/settings-persistence-on-shutdown branch from 0ab10fa to 0933b37 Compare April 3, 2026 10:55
@RaoufGhrissi RaoufGhrissi marked this pull request as ready for review April 3, 2026 10:56
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 3, 2026

Greptile Summary

This PR fixes settings data loss on ungraceful shutdown (SIGTERM/SIGKILL) by setting self.commit = true in the SetKeyValue and DeleteKeyValue command handlers. The change causes the worker's inner loop to break and call tx.commit() immediately after each settings write, instead of deferring up to the 15-second commit_interval_passed window. The approach is consistent with how CreateBucket and DeleteBucket already behave in the same file.

Confidence Score: 5/5

Safe to merge — minimal, focused, two-line logic change with no risk of regression.

The change is identical in structure to the already-correct CreateBucket/DeleteBucket handlers. No new state is introduced and the commit flag is properly reset at the top of each outer loop iteration (line 169), so there is no risk of spurious commits on subsequent unrelated requests. No P0 or P1 findings.

No files require special attention.

Important Files Changed

Filename Overview
aw-datastore/src/worker.rs Adds self.commit = true to SetKeyValue and DeleteKeyValue handlers, consistent with how CreateBucket/DeleteBucket already work, so a transaction commit is triggered immediately after settings writes rather than waiting up to 15 seconds.

Sequence Diagram

sequenceDiagram
    participant Client
    participant DatastoreWorker
    participant SQLite

    Client->>DatastoreWorker: SetKeyValue / DeleteKeyValue
    DatastoreWorker->>SQLite: insert_key_value / delete_key_value (within tx)
    DatastoreWorker->>DatastoreWorker: self.commit = true
    DatastoreWorker-->>Client: Response::Empty()
    Note over DatastoreWorker: Inner loop checks self.commit == true → break
    DatastoreWorker->>SQLite: tx.commit() [immediate]
    Note over DatastoreWorker: Before fix: waited up to 15 s for commit_interval_passed
Loading

Reviews (1): Last reviewed commit: "feat(datastore): ensure settings changes..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant