Skip to content

Conversation

@ronen-fr
Copy link
Contributor

@ronen-fr ronen-fr commented Jan 10, 2024

This is a partial implementation of a change in the way scrub resource reservation
requests (sent by a primary to its replicas) are handled. In the complete implementation,
scrub reservation requests are no longer granted or refused immediately, but rather are queued in an
asynch reserver (similar to the way backfill reservations are handled).

In this partial implementation - all requests are treated as 'legacy', i.e. - the reserver
is never actually used. But the scrubber state-machine is already modified to handle
asynch requests.

@ronen-fr ronen-fr force-pushed the wip-rf-reserver branch 2 times, most recently from 1c1d095 to 1b3875c Compare January 11, 2024 11:38
@ronen-fr ronen-fr marked this pull request as ready for review January 11, 2024 11:38
@ronen-fr ronen-fr requested a review from a team as a code owner January 11, 2024 11:38
@ronen-fr ronen-fr requested a review from athanatos January 11, 2024 11:38
Copy link
Contributor

@athanatos athanatos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

* - 'cancel' from the primary. We clear our reservation state, and transition
* back to ReplicaUnreserved.
* - a chunk request: best response is debateable. For now - we treat that
* as a cancellation followed by the chunk request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should handle the chunk response without affecting the reservation machinery at all. It should be impossible, but it's not a problematic state for the replica so I don't think it's worth adding special code for. It might be worth logging so that teuthology can fail the test, though.

*
* - a new reservation request for the same PG: which means we had missed the
* previous cancellation request. Possible reactions: asserting, or simply
* removing the previous request from the queue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should cancel the previous one and start the new one. It should also log so that teuthology can fail the test since this should be impossible.

@github-actions github-actions bot added the tests label Jan 14, 2024
@ronen-fr ronen-fr force-pushed the wip-rf-reserver branch 2 times, most recently from 778d173 to e93f680 Compare January 14, 2024 15:32
@github-actions

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@ronen-fr ronen-fr force-pushed the wip-rf-reserver branch 3 times, most recently from 2b663d8 to 8311289 Compare January 26, 2024 12:31
To be used when handling replica reservation requests from "old"
primaries, that expect an immediate grant/deny reply.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
Up-to-date primaries will set this flag when sending a reservation
request. The replica OSD, if too busy to handle the request immediately, will queue
it until such time that the number of concurrent reservations is below the
configured limit. The queued requests are honored in FIFO order.

Old primaries will not set this flag, and will receive the expected
grant or deny reply immediately.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
The scrub async reserver is not yet used. All requests are treated as
'legacy' requests, i.e. requests that expect an immediate grant/deny
reply.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
@ronen-fr
Copy link
Contributor Author

jenkins retest this please

@ronen-fr
Copy link
Contributor Author

jenkins test api

@ronen-fr
Copy link
Contributor Author

jenkins test make check

@ronen-fr
Copy link
Contributor Author

Merging based on multiple Teuthology runs

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants