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

Fix for cyclemanager not resetting unregisterRequested flag #4420

Merged
merged 3 commits into from
Mar 8, 2024

Conversation

aliszka
Copy link
Member

@aliszka aliszka commented Mar 7, 2024

What's being changed:

Addresses: #4418

Callback registered in cyclecallbackgroup can be unregistered or deactivated. Both of this actions used to overwrite shouldAbort function passed to callback to always return positive value, indicating that long-running callback should abort its execution whenever possible. Overwritten shouldAbort function was never restored after callback was activated. As a result callback once deactivated was never successfully activated again.

To ensure all data already sent to database are included into backup, they have to persisted in files. Therefore memtables are forced to flush. Forced flush requires deactivating and activating back again background flush process responsible for flushing dirty memtables in intervals.
Due to bug described above, background flush process was not correctly resumed, resulting in data being stored in memory and wals until db shutdown (forced flush) or db restart (wal ingestion).

This fix makes sure shouldAbort function is restored to its original form when callback is activated after being deactivated.

Review checklist

  • Documentation has been updated, if necessary. Link to changed documentation:
  • Chaos pipeline run or not necessary. Link to pipeline: [TEST] cyclemanager fix weaviate-chaos-engineering#186
  • All new code is covered by tests where it is reasonable.
  • Performance tests have been run or not necessary.

@aliszka aliszka changed the title fix wip1 Fix for cyclemanager not resetting shouldAbort flag Mar 7, 2024
@aliszka aliszka marked this pull request as ready for review March 7, 2024 23:31
@aliszka aliszka changed the title Fix for cyclemanager not resetting shouldAbort flag Fix for cyclemanager not resetting unregisterRequested flag Mar 7, 2024
@aliszka aliszka changed the base branch from stable/v1.24 to stable/v1.23 March 8, 2024 11:24
@aliszka aliszka changed the base branch from stable/v1.23 to stable/v1.24 March 8, 2024 11:29
@aliszka aliszka changed the base branch from stable/v1.24 to stable/v1.23 March 8, 2024 11:29
@aliszka aliszka changed the base branch from stable/v1.23 to stable/v1.24 March 8, 2024 11:29
@aliszka aliszka changed the base branch from stable/v1.24 to stable/v1.23 March 8, 2024 11:29
@aliszka aliszka closed this Mar 8, 2024
@aliszka aliszka reopened this Mar 8, 2024
Copy link

sonarcloud bot commented Mar 8, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
66.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@parkerduckworth parkerduckworth merged commit 44e81ca into stable/v1.23 Mar 8, 2024
21 of 23 checks passed
@parkerduckworth parkerduckworth deleted the fix_cyclemanager_activate branch March 8, 2024 14:40
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.

None yet

3 participants