Add seshat:fold/3 to fold over all ids in a group#17
Open
the-mikedavis wants to merge 1 commit intorabbitmq:mainfrom
Open
Add seshat:fold/3 to fold over all ids in a group#17the-mikedavis wants to merge 1 commit intorabbitmq:mainfrom
seshat:fold/3 to fold over all ids in a group#17the-mikedavis wants to merge 1 commit intorabbitmq:mainfrom
Conversation
This can be used to efficiently query for specific counters across all `id`s in a `group`. Equivalently you could use `counters/1` and then `maps:fold/3`, however this `counters/1` creates a map of maps with all counters - this introduces a lot of intermediary garbage.
the-mikedavis
commented
Dec 17, 2025
Contributor
Author
the-mikedavis
left a comment
There was a problem hiding this comment.
I was imagining that this could be nice for rabbitmq/rabbitmq-server#15098. With rabbitmq/osiris#197 we could efficiently find all writers with replicas that are behind, something like:
seshat:fold(
fun ({osiris_writer, SQ}, CRef, Acc) ->
case counters:get(CRef, 9) > MaxDiff of
true ->
[SQ | Acc];
false ->
Acc
end;
(_, _, Acc) ->
Acc
end, [], osiris).
Comment on lines
+204
to
+207
| fold(Fun, Acc0, Group) -> | ||
| ets:foldl(fun(#entry{id = Id, cref = CRef}, Acc) -> | ||
| Fun(Id, CRef, Acc) | ||
| end, Acc0, seshat_counters_server:get_table(Group)). |
Contributor
Author
There was a problem hiding this comment.
API-wise, passing the counters ref feels wrong. Ideally we could take advantage of the #entry.field_spec here so that the fold function could use the field name atoms instead of indexes.
Maybe we can pass in a function? Something like:
fold(Fun, Acc0, Group) ->
ets:foldl(fun(#entry{id = Id,
field_spec = FieldsSpec,
cref = CRef}, Acc) ->
Fields = resolve_fields_spec(FieldsSpec),
GetCounterFun = fun(Field) ->
%% lists:keyfind/3 the field and use
%% its index with counters:get/2
end,
Fun(Id, GetCounterFun, Acc)
end, Acc0, seshat_counters_server:get_table(Group)).
Contributor
Author
There was a problem hiding this comment.
Also - seems like it might be valuable to make fields_spec() a map keyed by name? I think that would benefit a few of the functions in this module
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This can be used to efficiently query for specific counters across all
ids in agroup. Equivalently you could usecounters/1and thenmaps:fold/3, however thiscounters/1creates a map of maps with all counters - this introduces a lot of intermediary garbage.