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

Jump to bottom

Compaction manager "periodic reevaluation" is one-off #13430

Closed
raphaelsc opened this issue Apr 3, 2023 · 4 comments · Fixed by #13431
Closed

Compaction manager "periodic reevaluation" is one-off #13430

raphaelsc opened this issue Apr 3, 2023 · 4 comments · Fixed by #13431
Assignees
@raphaelsc
Labels
area/compaction
Milestone
5.3

Comments

@raphaelsc
Copy link
Member

The manager intended to periodically reevaluate compaction need for each registered table. But it's not working as intended. The reevaluation is one-off.

This means that compaction was not kicking in later for a table, with low to none write activity, that had expired data 1 hour from now.
@raphaelsc raphaelsc self-assigned this Apr 3, 2023
raphaelsc added a commit to raphaelsc/scylla that referenced this issue Apr 3, 2023
@raphaelsc
compaction: Make compaction reevaluation actually periodic
eb5e3c8 
The manager intended to periodically reevaluate compaction need for
each registered table. But it's not working as intended.
The reevaluation is one-off.

This means that compaction was not kicking in later for a table, with
low to none write activity, that had expired data 1 hour from now.

Also make sure that reevaluation happens within the compaction
scheduling group.

Fixes scylladb#13430.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
@raphaelsc raphaelsc mentioned this issue Apr 3, 2023
Merged
@mykaul mykaul added this to the 5.3 milestone Apr 3, 2023
raphaelsc added a commit to raphaelsc/scylla that referenced this issue Apr 3, 2023
@raphaelsc
compaction: Make compaction reevaluation actually periodic
c3176e7 
The manager intended to periodically reevaluate compaction need for
each registered table. But it's not working as intended.
The reevaluation is one-off.

This means that compaction was not kicking in later for a table, with
low to none write activity, that had expired data 1 hour from now.

Also make sure that reevaluation happens within the compaction
scheduling group.

Fixes scylladb#13430.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
denesb added a commit that referenced this issue Apr 5, 2023
@denesb
Merge 'Compaction reevaluation bug fixes' from Raphael "Raph" Carvalho
9a02315 
A problem in compaction reevaluation can cause the SSTable set to be left uncompacted for indefinite amount of time, potentially causing space and read amplification to be suboptimal.

Two revaluation problems are being fixed, one after off-strategy compaction ended, and another in compaction manager which intends to periodically reevaluate a need for compaction.

Fixes #13429.
Fixes #13430.

Closes #13431

* github.com:scylladb/scylladb:
  compaction: Make compaction reevaluation actually periodic
  replica: Reevaluate regular compaction on off-strategy completion
syuu1228 pushed a commit to syuu1228/scylla that referenced this issue Apr 13, 2023
@raphaelsc @syuu1228
compaction: Make compaction reevaluation actually periodic
2bc105d 
The manager intended to periodically reevaluate compaction need for
each registered table. But it's not working as intended.
The reevaluation is one-off.

This means that compaction was not kicking in later for a table, with
low to none write activity, that had expired data 1 hour from now.

Also make sure that reevaluation happens within the compaction
scheduling group.

Fixes scylladb#13430.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
denesb added a commit that referenced this issue Apr 19, 2023
@denesb
Merge 'Compaction reevaluation bug fixes' from Raphael "Raph" Carvalho
da9f903 
A problem in compaction reevaluation can cause the SSTable set to be left uncompacted for indefinite amount of time, potentially causing space and read amplification to be suboptimal.

Two revaluation problems are being fixed, one after off-strategy compaction ended, and another in compaction manager which intends to periodically reevaluate a need for compaction.

Fixes #13429.
Fixes #13430.

Closes #13431

* github.com:scylladb/scylladb:
  compaction: Make compaction reevaluation actually periodic
  replica: Reevaluate regular compaction on off-strategy completion

(cherry picked from commit 9a02315)
@mykaul
Copy link
Contributor

mykaul commented May 8, 2023

@raphaelsc - do we need a backport to 5.1, 5.0 ?

@avikivity
Copy link
Member

@denesb I see you backported to 5.2, but why not to 5.1 too?

@raphaelsc
Copy link
Member Author

@denesb I see you backported to 5.2, but why not to 5.1 too?

I need to resolve a conflict. Will do it early this week

raphaelsc added a commit to raphaelsc/scylla that referenced this issue May 24, 2023
@raphaelsc
compaction: Make compaction reevaluation actually periodic
6cdd5cc 
The manager intended to periodically reevaluate compaction need for
each registered table. But it's not working as intended.
The reevaluation is one-off.

This means that compaction was not kicking in later for a table, with
low to none write activity, that had expired data 1 hour from now.

Also make sure that reevaluation happens within the compaction
scheduling group.

Fixes scylladb#13430.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
(cherry picked from commit 156ac0a)
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
@raphaelsc raphaelsc mentioned this issue May 24, 2023
Merged
denesb added a commit that referenced this issue May 25, 2023
@denesb
Merge 'Backport compaction reevaluation fixes to branch-5.1' from Rap…
cfa8fa1 
…hael "Raph" Carvalho

Fixes #13429.
Fixes #12390.
Fixes #13430.

Closes #14009

* github.com:scylladb/scylladb:
  compaction: Make compaction reevaluation actually periodic
  compaction_manager: Fix reactor stalls during periodic submissions
  compaction_manager: reindent postponed_compactions_reevaluation()
  compaction_manager: coroutinize postponed_compactions_reevaluation()
  compaction_manager: make postponed_compactions_reevaluation() return a future
  replica: Reevaluate regular compaction on off-strategy completion
@DoronArazii
Copy link

Backported.
Removing label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@raphaelsc raphaelsc

Labels
area/compaction
Projects
None yet
Milestone
5.3
Development

Successfully merging a pull request may close this issue.

Compaction reevaluation bug fixes raphaelsc/scylla
6 participants
@avikivity @raphaelsc @mykaul @bhalevy @scylladb-promoter @DoronArazii