Fix for task distribution in cluster requests. #1178
Conversation
|
Workflow [PR], commit [cb8916c0] |
…ge_cluster_distribution
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| log, | ||
| "Iterator exhausted. Assigning unprocessed file {} to replica {}", | ||
| "Iterator exhausted. Assigning unprocessed file {} to replica {}, original matching replica {}", | ||
| file_path, | ||
| number_of_current_replica | ||
| number_of_current_replica, | ||
| it->second.second |
There was a problem hiding this comment.
In getAnyUnprocessedFile the iterator it is erased from unprocessed_files earlier in this block (line 227), but the log message still reads it->second.second afterwards. Erasing an iterator from an unordered_map invalidates it, so dereferencing it for logging is undefined behaviour and can lead to crashes or garbage output whenever this branch runs. Capture the original replica id before erasing the entry and log that instead.
Useful? React with 👍 / 👎.
|
@codex review |
|
|
||
| # New node and old node, different nodes order | ||
| # New replica and old replica, different replicas order | ||
| # All files form removed replica changed replica |
There was a problem hiding this comment.
Nitpick: form -> from here and the line below
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix for task distribution in cluster requests.
Documentation entry for user-facing changes
When
lock_object_storage_task_distribution_mssetting is greater than 0, replica, which processed all tasks can get some unprocessed task, when "replica-task owner" does not ask for a new task for this time.But on start of query if one replica has a few tasks (less than
max_threads), it can ask for unprocessed task before other replica ask for first task. In this case task goes to the "wrong" replica even when "correct" is ready to process it.This fix add time of query start as initial time for replicas activity. This allow to send unprocessed tasks to random replicas only
lock_object_storage_task_distribution_msafter query start.Second fix - sending JSON with waiting time was broken during rebase, as result replica gets empty string what means "no more tasks to process"
Third fix - selection from
unprocessed_filesdoes not see on priority replica, as result tasks fromunprocessed_filesgoes to random replica.CI/CD Options
Exclude tests:
Regression jobs to run: