You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "nickva (via GitHub)" <gi...@apache.org> on 2026/06/10 22:04:33 UTC
[PR] Improve document grouping in fabric_doc_update [couchdb]
nickva opened a new pull request, #6032:
URL: https://github.com/apache/couchdb/pull/6032
The idea of doc/shard grouping and rotation was that different databases would start their workers on different copies, spread those uniformly and consistently by `{Range, DbName}`.
However, the order of entries were actually dependent on `dict:to_list/1` internals. So, if `dict` was changed to return sorted items, like for instance maps do, we'd end up suddently breaking the supposedly randomized spread (I found this out by actually trying to use a map there instead of a dict).
To fix this, make the rotation explicit. Group the entries first, then sort each group, and then rotate by `{Range, DbName}`.
While at it make it so we don't have to duplicate the main rotate function in tests and use the actual function we intend to test.
Add some explicit tests for this behavior. One downside is those tests rely on the exact rotate_list/2 implementation (`crc32(?term_to_binary/1`) so if that changes we'll know and have to update the test.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve document grouping in fabric_doc_update [couchdb]
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva merged PR #6032:
URL: https://github.com/apache/couchdb/pull/6032
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve document grouping in fabric_doc_update [couchdb]
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on code in PR #6032:
URL: https://github.com/apache/couchdb/pull/6032#discussion_r3414531611
##########
src/fabric/src/fabric_doc_update.erl:
##########
@@ -339,32 +339,38 @@ good_reply(_, _) ->
-spec group_docs_by_shard(binary(), [#doc{}]) -> [{#shard{}, [#doc{}]}].
group_docs_by_shard(DbName, Docs) ->
+ Grouped = group_docs([{Doc, mem3:shards(DbName, Id)} || #doc{id = Id} = Doc <- Docs]),
Review Comment:
Hmm it's one ets lookup per doc id. But we can probably add a helper function to get all the shards and then do `mem3:shards(Db, DocId, Shards)` we have all the bits in mem3 just need to export it I think
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve document grouping in fabric_doc_update [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on code in PR #6032:
URL: https://github.com/apache/couchdb/pull/6032#discussion_r3414346509
##########
src/fabric/src/fabric_doc_update.erl:
##########
@@ -339,32 +339,38 @@ good_reply(_, _) ->
-spec group_docs_by_shard(binary(), [#doc{}]) -> [{#shard{}, [#doc{}]}].
group_docs_by_shard(DbName, Docs) ->
+ Grouped = group_docs([{Doc, mem3:shards(DbName, Id)} || #doc{id = Id} = Doc <- Docs]),
Review Comment:
not a huge concern but we'll call mem3:shards/2 quite often here, would it be possible to get the shard list once (mem3:shards/1) and reuse that list. I'm not as much concerned about performance as the weird case if the result of mem3:shards/2 happens to change during this loop (if a shard split etc is happening).
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve document grouping in fabric_doc_update [couchdb]
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on code in PR #6032:
URL: https://github.com/apache/couchdb/pull/6032#discussion_r3414947919
##########
src/fabric/src/fabric_doc_update.erl:
##########
@@ -339,32 +339,38 @@ good_reply(_, _) ->
-spec group_docs_by_shard(binary(), [#doc{}]) -> [{#shard{}, [#doc{}]}].
group_docs_by_shard(DbName, Docs) ->
+ Grouped = group_docs([{Doc, mem3:shards(DbName, Id)} || #doc{id = Id} = Doc <- Docs]),
Review Comment:
Updated with a `mem3:shards(Db, DocId, Shards)` helper to work on a snapshot of shards. So it will either go to the old version of the shardmap or the new one and should be bit faster since it's one shardmap ets table read, one hash function lookup then it's local operations. Before it was a lookup per docid
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org