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

[ServiceBus] Improve auto lock renewer implementation #20071

Merged

Conversation

yunhaoling
Copy link
Contributor

@yunhaoling yunhaoling commented Aug 3, 2021

addressing issue: #19362

currently sync auto lock renewer is implemented the way that each thread in the ThreadPoolExecutor handles lock renewal for one message/session at a time.

This could bring trouble when the amount of messages needed to be auto lock renewed is greater than the max workers of the executor -- E.g., executor has 4 workers, user register 10 messages, only the first 4 messages will be auto lock renewed.

This PR has proposed a new approach to support messages amount > max workers amount which is described as following:

The overall design is to decouple the renew process via a queue so that worker threads could keep popping and handling renew tasks from the queue.

The algorithm is:

  • a: If users pass the max worker
    • a-1: if max worker is 1, then we have a single thread, which keeps pop renew task from -> check -> renew if needed -> push back renew tasks
    • a-2: if max worker is None or > 1, then the model is we have 1 dispatching thread, and (max_worker - 1) renew worker threads. The dispatching thread keeps popping task meta info from the queue and submit them to the executor. The worker thread do the renew work if needed and push the task meta info back into the queue if the object is still renewable.
  • b: If users pass the executor, there's no public api in python to tell us the max worker value of an executor. we have to infer whether max_worker > 1 and the dispatching model could be applied. A inferring process will be started at the very beginning.
    • the inferring process would submit task within a submitted task, and then wait on the task for infer_timeout seconds. If max_worker > 1, then the sub-submitted task could finish,else waiting for the task would result in tineout error indicating that there's just 1 max_worker.
    • with the inferred max worker info, we could go to part a.

@ghost ghost added the Service Bus label Aug 3, 2021
@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling yunhaoling marked this pull request as ready for review August 5, 2021 18:21
@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@annatisch annatisch left a comment

Choose a reason for hiding this comment

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

LGTM :)

@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@swathipil swathipil left a comment

Choose a reason for hiding this comment

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

lgtm!

@yunhaoling yunhaoling merged commit 3386082 into Azure:main Aug 7, 2021
@yunhaoling yunhaoling deleted the yuling/sb/autolockrenewer-improvement branch August 7, 2021 04:31
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.

3 participants