Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer read replicas when choosing nodes to search #1768

Merged

Conversation

jotare
Copy link
Contributor

@jotare jotare commented Jan 24, 2024

Description

Change logic around choose_node so read replicas are preferred over primary index nodes

Potential improvements

With this logic, if we call choose_node(shard, read_only=True) and there's 1 read replica and 2 primary nodes, searches will always go to the secondary. Maybe a better load balancing can be thought.

How was this PR tested?

Unit tests

@jotare jotare requested a review from a team January 24, 2024 15:13
Copy link

This pull request has been linked to Shortcut Story #8610: [Replication] Use secondaries first.

Copy link

codecov bot commented Jan 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2ab2303) 82.19% compared to head (c54ac3f) 82.17%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1768      +/-   ##
==========================================
- Coverage   82.19%   82.17%   -0.03%     
==========================================
  Files         336      336              
  Lines       19852    19829      -23     
==========================================
- Hits        16317    16294      -23     
  Misses       3535     3535              
Flag Coverage Δ
ingest 69.04% <ø> (ø)
nucliadb 70.35% <100.00%> (-0.05%) ⬇️
reader 79.82% <ø> (ø)
sdk 40.64% <ø> (ø)
search 74.60% <ø> (ø)
standalone 88.29% <ø> (ø)
train 63.60% <ø> (ø)
utils 81.62% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@lferran lferran left a comment

Choose a reason for hiding this comment

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

Looks great! Just left one comment!

Would be nice to catch errors when querying a secondary and fallback to a primary. But maybe in a separate PR

@jotare jotare force-pushed the joanantoniriera4168/sc-8610/replication-use-secondaries-first branch from eb0b38e to e721c2e Compare January 29, 2024 10:48
@jotare jotare merged commit 1f3d8b1 into main Jan 29, 2024
83 checks passed
@jotare jotare deleted the joanantoniriera4168/sc-8610/replication-use-secondaries-first branch January 29, 2024 17:33
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