refactor: simplify app_config service and fix race conditions #267

New Issue

AtHeartEngineer · 2025-07-08T08:44:02-04:00

AtHeartEngineer commented

2025-07-08 08:44:02 -04:00

Originally created by @ItalyPaleAle on 4/9/2025

Fixes #391
Fixes #407

As discussed with @stonith404 , this PR refactors the app_config service with the goals of greatly simplify the logic, and fixing race conditions.

There are significant performance improvements too, especially when updating config, as the number of database round-trips has been reduced to no more than 2 (currently, it's 2 per each key). API calls to retrieve config are also much faster as they return the data from memory only.

Stores the config object in-memory in an atomic.Pointer, which removes the risk of data races.
Ensures consistency when updating config in the database, by using a transaction and acquiring a table-level lock. This ensures that config is only written by a single goroutine.
Simplifies the data structures a lot. In particular, default values and attributes (such as marking fields as non-public) can are not stored in the database anymore, but only in the code. This reduces the complexity of keeping all data in-sync, since it wasn't necessary to have that info int eh DB.

PR includes extensive unit tests which use an in-memory SQLite database to validate the behavior of the service.

Note: I had to update to Go 1.24 to be able to use testing.Context().

*Originally created by @ItalyPaleAle on 4/9/2025* Fixes #391 Fixes #407 As discussed with @stonith404 , this PR refactors the app_config service with the goals of greatly simplify the logic, and fixing race conditions. There are significant performance improvements too, especially when updating config, as the number of database round-trips has been reduced to no more than 2 (currently, it's 2 per each key). API calls to retrieve config are also much faster as they return the data from memory only. 1. Stores the config object in-memory in an `atomic.Pointer`, which removes the risk of data races. 2. Ensures consistency when updating config in the database, by using a transaction and acquiring a table-level lock. This ensures that config is only written by a single goroutine. 3. Simplifies the data structures a lot. In particular, default values and attributes (such as marking fields as non-public) can are not stored in the database anymore, but only in the code. This reduces the complexity of keeping all data in-sync, since it wasn't necessary to have that info int eh DB. PR includes extensive unit tests which use an in-memory SQLite database to validate the behavior of the service. Note: I had to update to Go 1.24 to be able to use `testing.Context()`.

AtHeartEngineer closed this issue

2025-07-08 08:44:02 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github/pocket-id#267