Skip to content

Build ring before publishing pool strategy - #77

Merged
lpgauth merged 1 commit into
masterfrom
fix/ring-built-before-strategy-publish
Aug 4, 2026
Merged

Build ring before publishing pool strategy#77
lpgauth merged 1 commit into
masterfrom
fix/ring-built-before-strategy-publish

Conversation

@lpgauth

@lpgauth lpgauth commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Summary

marina_pool:sync/3 inserted {token_aware, N} into the foil table before marina_ring:build/1 had compiled marina_ring_utils. In that window, a query carrying a routing key resolved the token_aware strategy and called marina_ring_utils:lookup/1 on a module that did not exist yet, crashing the caller with undef instead of getting {error, marina_pool_not_started}.

This reordering compiles the ring first, so the strategy only becomes visible to callers once the lookup module is loaded. The same window reopened on every {topology_full_sync, _} rebuild, not just at startup; both are covered. Building the ring first has no dependency on the node index — marina_compiler needs only the token list and the pure marina_pool:node_id/1, and pools are already started by that point.

No hot-path cost: the fix is pure ordering, with no guard added around the ring lookup itself.

marina_pool:sync/3 inserted {token_aware, N} into the foil table
before marina_ring:build/1 had compiled marina_ring_utils. A query
carrying a routing key in that window resolved the token_aware
strategy and called marina_ring_utils:lookup/1 on a module that did
not exist yet, crashing the caller with undef instead of getting
{error, marina_pool_not_started}.

Compile the ring first, so the strategy only becomes visible to
callers once the lookup module is loaded. This also applies to
topology re-syncs, where the same window reopened on every rebuild.
@lpgauth
lpgauth merged commit e1fa22e into master Aug 4, 2026
1 check passed
@lpgauth lpgauth mentioned this pull request Aug 4, 2026
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.

1 participant