blackwell-systems opened a new issue, #6170:
URL: https://github.com/apache/couchdb/issues/6170
### Version
unknown (main). From 9af4ae4c ("Fix revision patching for replicated
changes", #6168); tested at 9af4ae4c and 5b14b7c. Not in 3.5.3.
### Describe the problem you're encountering
With `[couchdb] stem_interactive_updates = false`, three replicated writes
(`_bulk_docs` with `new_edits: false`) leave a document with a leaf whose value
is `?REV_MISSING`. After that:
- `GET /db/X` returns 500 (`function_clause` in `couch_doc:rev_info/1`,
reached from `to_doc_info_path/1`). An interactive `PUT` to X also returns 500.
- `GET /db/_changes` returns 500 for any feed that reaches X's update,
including the full feed from `since=0`.
- Views on the database return 500, and replicating from it fails with
`changes_reader_died`.
- Compaction crashes on the same leaf (`function_clause` in
`couch_bt_engine_compactor:copy_docs/4`), so the database can no longer be
compacted.
- `_all_docs` has no row for X, because `couch_doc:is_deleted/1` does not
count the `[]` leaf as live.
Setting `stem_interactive_updates` back to true does not clear it. A later
replicated write to X with stemming on does.
Tested with the same toolchain, before and after the commit:
| Commit | GET X | `_changes` | Compaction |
|---|---|---|---|
| d3884102 (parent of 9af4ae4c) | 200 | 200 | completes and folds the
duplicate (GET then 404, deleted) |
| 9af4ae4c | 500 | 500 | crashes |
| 5b14b7c | 500 | 500 | crashes |
It is not tied to tiny revision histories: the same three writes with a
998-revision history for the deleted branch (revisions 3 to 1000, against a
full history from revision 1) give the same results.
### Expected Behaviour
The same result as with the default setting: one path `a -> b -> c -> d`
with `4-d` deleted as the only leaf, so `GET /db/X` returns 404 (deleted), and
`_changes`, views and compaction work.
### Steps to Reproduce
Single node, plain database, no VDU. Needs `curl`. The flag is set on
`_node/_local` only, so on a multi-node `./dev/run` cluster set it on every
node (or use `./dev/run -n 1`).
```sh
COUCH=${COUCH:-http://admin:[email protected]:5984}
DB=r3
A=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; B=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
C=cccccccccccccccccccccccccccccccc; D=dddddddddddddddddddddddddddddddd
curl -sS -X PUT
"$COUCH/_node/_local/_config/couchdb/stem_interactive_updates" \
-H 'X-Couch-Persist: false' -d '"false"'
curl -sS -X DELETE "$COUCH/$DB" > /dev/null
curl -sS -X PUT "$COUCH/$DB"
put() {
curl -sS -X POST "$COUCH/$DB/_bulk_docs" -H 'Content-Type:
application/json' \
-d "{\"new_edits\":false,\"docs\":[$1]}"; echo
}
put
'{"_id":"X","_rev":"1-'$A'","_revisions":{"start":1,"ids":["'$A'"]},"v":1}'
put
'{"_id":"X","_rev":"4-'$D'","_deleted":true,"_revisions":{"start":4,"ids":["'$D'","'$C'"]}}'
put
'{"_id":"X","_rev":"3-'$C'","_revisions":{"start":3,"ids":["'$C'","'$B'","'$A'"]},"v":3}'
curl -sS -o /dev/null -w '%{http_code}\n' "$COUCH/$DB/X" # 500 on
main, 200 on d3884102
curl -sS -o /dev/null -w '%{http_code}\n' "$COUCH/$DB/_changes" # 500 on
main, 200 on d3884102
curl -sS -X DELETE
"$COUCH/_node/_local/_config/couchdb/stem_interactive_updates" \
-H 'X-Couch-Persist: false'
```
### Your Environment
Built from source, Erlang/OTP 28.5, `./configure --dev --js-engine=quickjs`,
single node, macOS arm64. Reproduces at q=1, q=2 and q=8.
### Additional Context
From the code and a dump of the stored tree, this is what happens. After the
second write the tree is `a` and `c -> d`, where `c` is an internal node with
`?REV_MISSING`. In the third write, `replicated_path` calls
`get_full_key_paths(OldTree, [{3, c}])`, finds `c` inside the `c -> d` branch,
and maps the new path's leaf to that stored value. It assumes the merge lands
on the node it found, but `couch_key_tree:merge` extends the first branch it
can (`a`) and stops, so the value ends up on a different node: a leaf `3-c`
with no body, plus a duplicate `c`. With stemming on, `stem` throws `dupe_keys`
and `repair_tree` folds the duplicate into `a -> b -> c -> d`, which hides it.
With stemming off the updater never repairs it, and compaction, which does
stem, crashes on the bodyless leaf before it gets there.
<details>
<summary>Stored tree after each write
(<code>couch_db:get_full_doc_info</code>)</summary>
`missing` is `?REV_MISSING`, `body` a stored `#leaf{}`, `del` a deleted one.
```
main (5b14b7c) d3884102
write 1 [{1, a body}] [{1, a body}]
write 2 [{1, a body}, {3, c missing -> d del}] [{1, a body},
{3, c missing -> d del}]
write 3 [{1, a body -> b missing -> c missing}, [{1, a body ->
b missing -> c body},
{3, c missing -> d del}] {3, c missing
-> d del}]
leaves 4-d, 3-c; duplicate key {3, c} leaves 4-d,
3-c; duplicate key {3, c}
```
</details>
Before 9af4ae4c the flag-off tree already holds the duplicate. The shard has
two leaves, so `GET /db/X` returns live `3-c` while `open_revs=all` shows only
`4-d`, because fabric merges the two leaf paths. That lasts until the next
compaction folds it. 9af4ae4c makes the new leaf bodyless, which turns the
duplicate into a crash and blocks the compaction that would have repaired it.
Through the replicator, the third write can only reach a target that already
has `{3, c}` as a key by way of the `_revs_diff` skip in `find_missing`, the
same path as in #6162. I have only reproduced this with `_bulk_docs`.
Two approaches I tried locally:
- Making `replicated_path` skip the copy when the revision lands as a leaf
removes the 500, but with stemming on that leaf can be folded into an internal
node by `repair_tree` right after, so the check is not reliable inside
`replicated_path`.
- Leaving `replicated_path` as it is and folding duplicate keys in the
`false` branch of `maybe_stem_full_doc_info` (calling `couch_key_tree:stem/2`
with a depth limit of `16#7FFFFFFF`, which only falls into `repair_tree` when
there is a duplicate and trims no history) fixes it: the three writes end as `a
-> b -> c -> d` with `4-d` deleted as the only leaf. The change is confined to
the `false` branch, so default behaviour is unchanged. With it,
`couch_db_doc_tests` (which runs with the flag off),
`couchdb_update_conflicts_tests` and `couch_key_tree_tests` pass. It does walk
the tree on every write with the flag off, which the flag exists to avoid; a
narrower version would repair only when `replicated_path` found the revision
but the merge reported a new leaf rather than `internal_node`.
The flag is still read in `maybe_stem_full_doc_info` and set by
`couch_db_doc_tests`, and #3193 (fixed in 2.1) was an earlier bug with the flag
off. If the flag isn't worth keeping, removing it closes this too. If one of
the approaches above looks right, I can test it against these cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
