#50539 Issue 50538 - cleanAllRUV task limit is not enforced for replicated tasks
Closed by spichugi. Opened by mreynolds.
mreynolds/389-ds-base issue50538  into  master

Download 50539.patch

Bug Description:

There is a hard limit of 64 concurrent cleanAllRUV tasks, but this limit is only enforced when creating "new" tasks. It was not enforced when a task was received via an extended operation. There were also race conditions in the existing logic that allowed the array of cleaned rids to get corrupted . This allowed for a very large number of task threads to be created.

Fix Description:

Maintain a new counter to keep track of the number of clean and abort threads to make sure it never over runs the rid array buffers.

relates: https://pagure.io/389-ds-base/issue/50538

rebased onto 425ef509dae561a0cb20d663fb9a2f2c6f45f629

rebased onto 8272d387dd2469ee9db8b7c7752ff616bbcb9a7e

The patch looks good, I'm just a bit confused by the test case: the issue is about not limiting the number of tasks when received via replication ext ops, but if I read the test correctly it is testing the limit when directly creating tasks. Would we need to add eg 50 tasks to two masters and then see what happens ?

The patch looks good, I'm just a bit confused by the test case: the issue is about not limiting the number of tasks when received via replication ext ops, but if I read the test correctly it is testing the limit when directly creating tasks. Would we need to add eg 50 tasks to two masters and then see what happens ?

Yeah the testcase is not reproducing the exact issue I originally reported. That's because I found more general problems with the entire process (and it was after midnight when I was wrapping it up and got tired). I'll change the test case shortly....

ok, you have my ack for the patch

rebased onto 3575730916ad7c0a20773b1e13d9c9d32c581632

Fixed another race condition, and improved CI test, please review...

shouldn't the message be on M2 as well, could you query for active tasks and count them ?

ack again

shouldn't the message be on M2 as well, could you query for active tasks and count them ?

No, and in fact it was really hard to reproduce this in the first place because the tasks would normally finish too fast. So the tasks I create on M1 are not "force cleaning". So m3 and m4 are stopped. In this case m1 will not send out the cleanAllRUV extended op. So they just hang around doing nothing. On M2 the tasks are set to "force clean" so this allows M2's cleanAllRUV extended op to be sent to other replicas regardless if m3 or m4 are stopped. So only m1 will actually receive more than 64 tasks at one time

thanks for the explanation

thanks for the explanation

Thanks for the review!

Pull-Request has been merged by mreynolds

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3595

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

Metadata