Skip to content

Fix append_update_replies case clause in fabric_doc_update#5989

Merged
nickva merged 1 commit into
mainfrom
fix-parallel-spawn-model-for-update-docs
May 1, 2026
Merged

Fix append_update_replies case clause in fabric_doc_update#5989
nickva merged 1 commit into
mainfrom
fix-parallel-spawn-model-for-update-docs

Conversation

@nickva
Copy link
Copy Markdown
Contributor

@nickva nickva commented Apr 30, 2026

In serialize_worker_startup=false mode it was possible before to get a function_clause error in append_update_replies/3. The function_clause was because of an empty Docs list and a non-empty Replies list. That happened when we had at least two conflicted replies for a doc, one arriving and others in-flight. The arriving one would knock out the doc from the expected replies list, such that when the second reply arrived, it wouldn't match up with anything.

To fix this, keep track of conflicts separately and check that conflicted list before extra workers spawn, and also avoid altering the grouped docs structure for in-flight requests

Also fix another regression, and that's in case of sws=false to keep going and perform the quorum the way we did before sws feature. Previously, we only spawned the workers in parallel but still short-cut the quorum on a conflict.

@nickva nickva requested a review from rnewson April 30, 2026 04:25
@nickva nickva force-pushed the fix-parallel-spawn-model-for-update-docs branch from 7d253a1 to c2328b9 Compare April 30, 2026 16:43
@nickva nickva force-pushed the fix-parallel-spawn-model-for-update-docs branch from c2328b9 to 7a74ea8 Compare May 1, 2026 18:05
Comment thread src/fabric/src/fabric_doc_update.erl Outdated
Copy link
Copy Markdown
Member

@rnewson rnewson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer this, it's easier to reason about the conflict handling. My #5993 was focused on preserving the original behaviour but I agree with nick that it still isn't quite a match, so this way is better.

In `serialize_worker_startup=false` mode it was possible before to get a
function_clause error in `fabric_doc_update:append_update_replies/3`. The
function_clause was because of an empty `Docs` list and a non-empty `Replies`
list. That happened when we had at least two conflicted replies for a doc one
arriving, and others in-flight, the arriving one would knock out the doc from
the expected replies list, such that when the second reply arrived, it wouldn't
match up with anything.

To fix this, keep track of conflicts separately and check that conflicted list
before extra workers spawn, and also avoid altering the grouped docs structure
for in-flight requests

Also fix another regression, and that's in case of sws=false to keep going and
perform the quorum the way we did before sws feature. Previously, we only
spawned the workers in parallel but still short-cut the quorum on a conflict.
@nickva nickva force-pushed the fix-parallel-spawn-model-for-update-docs branch from 7a74ea8 to 0483ee5 Compare May 1, 2026 19:17
@nickva nickva merged commit 425d8aa into main May 1, 2026
59 checks passed
@nickva nickva deleted the fix-parallel-spawn-model-for-update-docs branch May 1, 2026 20:10
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.

2 participants