seastar icon indicating copy to clipboard operation
seastar copied to clipboard

inheriting_concrete_execution_stage doesn't support scheduling group renaming

Open eliransin opened this issue 1 year ago • 1 comments

This bug is present since the introduction of 4a082fc5590bebc07b2772ed5875f9d46edad054 there is a bug in the code which has two impacts and is related to the inheriting_concrete_execution_stage class.

  1. Renaming a scheduling group doesn't update the execution_stage stats labels and the stage's label is still named after the old scheduling group's name.
  2. It excepts for some calls after some rename/create sequences to scheduling group.

An example scenario (verified with a unit test):

  1. Create an inheriting_concrete_execution_stage name A
  2. Create a scheduling group sg1 with name X
  3. Call the inheriting_concrete_execution_stage under sg1
  4. Rename sg1 to Y - at that stage the stats are still labeled A_X (and not A_Y)
  5. Create a scheduling group sg2 with name X (same as the initial name of sg1)
  6. Call the inheriting_concrete_execution_stage under sg2 - the execution_stage_manage::register_execution_stage function will throw std::invalid_argument("Execution stage A_X already exists.")

An apparent quick fix would be to append the index of the scheduling group for the execution stage name, but it will only fix the exception and not the fact that the stats are wrong.

eliransin avatar Mar 02 '23 10:03 eliransin

@Jadw1 can you expose this via a sequence of CREATE SERVICE LEVEL/DROP SERVICE LEVEL statements? If yes, please could you write a sequence of steps to reproduce? 8 CREATE/DROP statements should be enough. If not, we must close this.

kostja avatar Mar 29 '24 14:03 kostja