Improve mem3 #5552
Merged
Improve mem3 #5552
Conversation
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
Previously, we handled the case of missing ets tables by catching `error:badarg` in places like `for_db/2`, but then forgot to handle it the lower level `maybe_spawn_shard_writer/3` function, so call was still crashing and making a mess in the logs. This is more apparent on startup with nodes being upgraded in a busy cluster.
jaydoane
approved these changes
May 29, 2025
The answer to the question if order is important in the comments is: "yes" for most of the workers. So fix their order and use the `rest_for_one` [1] strategy. * `mem3_events` gen_event should be started before all the others * `mem3_nodes` gen_server is needed so everyone can query `mem3:nodes()` from it * `mem3_sync_nodes` needs to run before `mem3_sync` and `mem3_sync_event` so they can both can call `mem3_sync_nodes:add/1` * `mem3_distribution` force connects nodes from `mem3:nodes()`, so start it before `mem3_sync` since `mem3_sync:initial_sync/0` expects the connected nodes to be there when calling `mem3_sync_nodes:add(nodes())` * `mem3_sync_event_listener` has to start after `mem3_sync` so it can all `mem3_sync:push/2` * `mem3_seeds` and `mem3_reshard_sup` can wait till the end as they will spawn background work that can go for a while: seeding system dbs from other nodes or running resharding jobs. [1] https://www.erlang.org/doc/system/sup_princ.html#rest_for_one
Previously, we logged these events as notice but let's lower them to info. On startup or node upgrades they'll always be logged and they are not that interesting just add noise. Also, the `cluster_unstable/1` callback in `rexi_server_mon` log was duplicated so remove one of them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Improve cluster startup behavior as three separate commits:
Sometimes if
mem3is not initialized yet butrexicould start calling us, and so we have to handlemem3_shardsets tables not being created yet. Previously, we handled the case of missing ets tables by catchingerror:badargin places likefor_db/2, but then forgot to handle it the lower levelmaybe_spawn_shard_writer/3function, so call was still crashing and making a mess in the logs. This is more apparent on startup with nodes being upgraded in a busy cluster.Improve mem3 supervisor. There was a question there left from years back if order is important, it turns out it is. So fix the order. See the reasoning in the commit message. Also since order is important we want to use a rest_for_one supervisor not a
one_for_one.Cleanup node startup logging a bit. We were emitting duplicate logs and also at level that's a bit higher than needed.