-
Notifications
You must be signed in to change notification settings - Fork 134
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
[Improvement] Avoid unnecessary retry in some access checker when using DelegationShuffleManager #151
Comments
cc @jerqi |
@smallzhongfeng What do you think? |
I think this is a parameter setting. If the |
Firstly i think your PR is meaningful for cluster load access checker. But in other access checker, sometimes we needn’t retry. So I will introduce a special acess result of For exampleIf we enable two checkers
|
A little complex, are there similar mechanisms in the other systems? In my opinion, we shouldn't retry when we use candidate checker. when we only use health checker, we can retry, we can scale out our RSS at the same time. I doubt whether we need this mechanism? |
If not having this mechanism, how to handle the multiple checkers retry? In our internal env, we will use the multiple checkers, including health checker(need to retry) and customize checker(no need to retry). |
You can choose not to retry. |
No retry is OK. But this will not solve the problem described in the issue #127 |
Maybe you could choose not to retry when you use multiple checker, because in your description, the scenario of multiple checkers seems to be more dependent on the results of the candidates checker, in this way, it is not enough meaningful to retry, but the default checker is only |
But when having multiple checkers, and the apps are in candidates list, for these apps, it need retry. |
So do we need this feature ? cc @jerqi |
I don't think we need so complex retry mechanism ... |
OK. Close it. |
The retry mechanism is introduced by #127. But in some access checker, there's no need to retry, like candidates checker. But the health checker maybe need.
So I think we need introduce the
NON_TRANSIENT_ACCESS_DENIED
to avoid retry in some checker to reduce time.The text was updated successfully, but these errors were encountered: