Concurrent modification check should take place inside a database transaction
- Dominant language
- TypeScript
- Stars
- 129
- Forks
- 39
- Avg merge
- 1h 26m
- Merged PRs (30d)
- 2
Description
The check of the `LastUpdateUUID` currently performed by the `ensureNoConcurrentMod` function in `main.go` should be performed in the same database transaction as the actual write of the `Period`. The transaction should be committed only if the UUID on the incoming object matches the one in the database.
Under the current implementation, the check is performed outside the database write transaction. That means there is still a race condition whereby two updates to the same `Period` take place concurrently, both having a correct `LastUpdateUUID`. If both updates pass the `ensureNoConcurrentMod` check before either is written to the database, both write transactions will succeed, and both update requests should receive a successful HTTP response code, when in fact one write has stomped over the other one.
The same is true of the checks in `ensureTeamExistence` and `ensurePeriodExistence` as well, though race conditions here are not as harmful, as these checks are repeated (in the current `google_cds_store.go`) inside the database transaction. The same thing could be done for the UUID check (i.e. verifying it both in `main.go` and a database transaction), though it would be better to simply perform all the checks once in the database layer where possible, and make this part of the semantics of the `StorageService`.
This would likely require modifying the `StorageService` interface to return custom error codes, so that the different types of error can be distinguished in `main.go` and suitable HTTP status codes returned to the client in the different cases.
Contributor guide
Assessment
This issue has not been assessed yet.