Repository navigation
hack: repo-publish: refuse to commit over a racing publish - #77
martinpitt wants to merge 3 commits into
Conversation
`TxnCommit` only detects a racing writer that committed after `TxnStart`. Callers which compute new role data from an earlier read of the repository, such as merging the previous targets, have no way to tell that the repository moved on in between, and silently drop the other writer's changes. Add a `TxnOp` which fails with `ErrClobberedTransaction` unless the transaction starts from a given timestamp version. Timestamp versions identify a repository state, as `TxnCommit` never overwrites a versioned timestamp file. Signed-off-by: Martin Pitt <martin@amutable.com>
Expose `RequireTimestampVersion` so that scripts which compute targets before calling `repoctl snapshot` can refuse to commit over a repository state they did not compute them from. Signed-off-by: Martin Pitt <martin@amutable.com>
With `--merge`, repo-publish.sh reads the current targets from `timestamp.json`, merges them into the new targets, and only then runs `repoctl snapshot`. That starts the transaction and takes the timestamp ETag that `TxnCommit` compares against. A publish which commits between the merge and the snapshot is not detected: the snapshot commits successfully (based on the newer timestamp), but with targets merged from the older one, and the other publisher's targets are gone. `--no-merge` has the same problem whenever the given targets were derived from the repository's current state. Read `timestamp.json` once, merge from that copy, and pass its version to `repoctl snapshot --if-timestamp-version`, so that the snapshot fails instead. Signed-off-by: Martin Pitt <martin@amutable.com>
|
I'll take a proper look at this once I have some time later but |
|
Ah, it's because of |
|
@cyphar ah, I see! Do I get this right? Our infra still uses |
With
--merge, repo-publish.sh reads the current targets fromtimestamp.json, merges them into the new targets, and only then runsrepoctl snapshot. That starts the transaction and takes the timestamp ETag thatTxnCommitcompares against.A publish which commits between the merge and the snapshot is not detected: the snapshot commits successfully (based on the newer timestamp), but with targets merged from the older one, and the other publisher's targets are gone.
--no-mergehas the same problem whenever the given targets were derived from the repository's current state.I noticed that while evaluating what our infrastructure needs to do for supporting multiple architecture builds. That discovered this race condition. It will not actually affect our infra (at least not with the current plan), but this still seems worth addressing for other users.