Skip to content

Conversation

@xemul
Copy link
Contributor

@xemul xemul commented Sep 23, 2025

When a scheduling group is destroyed, the IO queue's priority_class_data
that corresponds to it stays intact. It's not actually a leak, as when
the reactor is destroyed all class data objects are collected.

However, if another scheduling group under the same ID is created, the
IO queue will re-use the dangling priority class for the newly created
group, but its name and shares won't match the new group until rename
or shares update. And it's not nice that IO class remains alive anyway.

Since scheduling group destruction must happen after all tasks from it
are completed and picked up, the class destruction code may also assume
that no queued or in-flight requests exist.

Test included.

@xemul xemul force-pushed the br-io-queue-destroy-classes-when-sched-group-dies branch from ed7f3b1 to ac30794 Compare September 23, 2025 17:12
@xemul xemul changed the title io_queue: Destroy priority class data whith scheduling group io_queue: Destroy priority class data with scheduling group Sep 23, 2025
When a scheduling group is destroyed, the IO queue's priority_class_data
that corresponds to it stays intact. It's not actually a leak, as when
the reactor is destroyed all class data objects are collected.

However, if another scheduling group under the same ID is created, the
IO queue will re-use the dangling priority class for the newly created
group, but its name and shares won't match the new group until rename
or shares update. And it's not nice that IO class remains alive anyway.

Since scheduling group destruction must happen after all tasks from it
are completed and picked up, the class destruction code may also assume
that no queued or in-flight requests exist.

Test included.

Signed-off-by: Pavel Emelyanov <[email protected]>
@xemul xemul force-pushed the br-io-queue-destroy-classes-when-sched-group-dies branch from ac30794 to f3df50c Compare September 29, 2025 13:31
@xemul
Copy link
Contributor Author

xemul commented Sep 29, 2025

upd:

  • assume that when priority class is destroyed, no queued or executing requests exist

@xemul xemul requested a review from avikivity September 29, 2025 13:51
@xemul
Copy link
Contributor Author

xemul commented Oct 7, 2025

@avikivity , please review

@avikivity avikivity merged commit 6bb1c8f into scylladb:master Oct 9, 2025
16 checks passed
@avikivity
Copy link
Member

I unmerged it, it needs rebasing after the data_sink API changes.

@xemul
Copy link
Contributor Author

xemul commented Oct 10, 2025

I don't see how it conflicts with data_sink API changes. Maybe it's API changes that need to be unmerged, because they clashed with #3018 's on counting_data_sink? OTOH, API level 8 in "library mode", i.e. without tests, demos and apps, still compiles fine

@xemul
Copy link
Contributor Author

xemul commented Oct 10, 2025

Created #3040

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