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

Assigner waits to get existing assignments from indexers #1116

Merged
merged 3 commits into from
Dec 10, 2022

Conversation

gammazero
Copy link
Collaborator

@gammazero gammazero commented Dec 9, 2022

Context

If an indexer is offline when the assigner service starts, the assigner cannot read the existing assignments or preferences from that indexer. The assigner will not know what publishers are assigned to that indexer, resulting in assigning those publishers to other indexers as announcements from them arrive.

The assigner service needs to retry getting indexer assignments before assigning a publisher to an indexer, since that publisher may already be assigned to an offline indexer. This prevents over-assigning publishers to the indexer pool.

Fixes #1118

Proposed Changes

If the assigner starts without getting assignment information for all indexers in the pool, then it will recheck each time an announce message is received from an unassigned peer. If there are still indexers offline, the assigner assumes that the peer may be assigned to an offline indexer and reduces the required number of assignments by the number of indexers offline.

This may mean that an unassigned peer will not get assigned until indexer(s) come back online. It is necessary to prevent over-assigning publishers to the indexer pool.

Tests

Tests include starting with an indexer offline and testing that:

  • Assignment information is initially unread
  • Publisher is not assigned when indexers assignments are unknown
  • After indexer is back online, all assignment info is all read when unassigned publisher seen

Revert Strategy

Reverting will return to the previous situation and may require resetting indexer assignments.

If an indexer is offline when the assigner service starts, the assigner cannot read the existing assignments or preferences from that indexer. The assigner will not know what publishers are assigned to that indexer, resulting in assigning those publishers to other indexers as announcements from them arrive.

The assigner service needs to retry getting indexer assignments before assigning a publisher to an indexer, since that publisher may already be assigned to an offline indexer. This prevents over-assigning publishers to the indexer pool.

If the assigner starts without getting assignment information for all indexers in the pool, then it will recheck each time an announce message is received from an unassigned peer. If there are still indexers offline, the assigner assumes that the peer may be assigned to an offline indexer and reduces the required number of assignments by the number of indexers offline.
@gammazero gammazero requested a review from masih December 9, 2022 23:29
@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2022

Codecov Report

Base: 54.67% // Head: 54.95% // Increases project coverage by +0.28% 🎉

Coverage data is based on head (f62249b) compared to base (0e2ed43).
Patch coverage: 79.45% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1116      +/-   ##
==========================================
+ Coverage   54.67%   54.95%   +0.28%     
==========================================
  Files         134      134              
  Lines       12877    12928      +51     
==========================================
+ Hits         7040     7105      +65     
+ Misses       5090     5080      -10     
+ Partials      747      743       -4     
Impacted Files Coverage Δ
assigner/core/assigner.go 78.25% <79.45%> (+1.28%) ⬆️
internal/ingest/ingest.go 72.15% <0.00%> (+0.13%) ⬆️
dagsync/subscriber.go 86.59% <0.00%> (+0.57%) ⬆️
internal/ingest/linksystem.go 71.03% <0.00%> (+0.59%) ⬆️
api/v0/admin/client/http/client.go 15.20% <0.00%> (+1.14%) ⬆️
dagsync/dtsync/sync.go 84.26% <0.00%> (+2.24%) ⬆️
dagsync/dtsync/syncer.go 84.04% <0.00%> (+3.19%) ⬆️
api/v0/httpclient/client.go 54.54% <0.00%> (+9.09%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@masih masih left a comment

Choose a reason for hiding this comment

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

Should we test this on dev first?

@gammazero
Copy link
Collaborator Author

Of course. I will deploy ther first. We should not see anything as this is really a measure to prevent problems if the assigner starts when indexers are offline.

@gammazero gammazero merged commit cf19d70 into main Dec 10, 2022
@gammazero gammazero deleted the assigner-waits-for-indexers branch December 10, 2022 07:45
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.

Do not assign publishers when pool assignments are unknown
3 participants