[Improvement] Avoid unnecessary retry in some access checker when using DelegationShuffleManager about incubator-uniffle HOT 14 CLOSED

cc @jerqi

from incubator-uniffle.

@smallzhongfeng What do you think?

from incubator-uniffle.

I think this is a parameter setting. If the retry interval * retry times is greater than the heartbeat time passed by the server to the coordinator, the check of AccessCandidatesChecker is meaningful. If theretry interval * retry times is less than the heartbeat time of the server, Can be solved by setting the number of retries to 0 ? @zuston

from incubator-uniffle.

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 NON_TRANSIENT_ACCESS_DENIED to avoid retry in some checkers.

For example

If we enable two checkers

1. when cluster-loader check deny but candidates checker pass, so we need to retry
2. when cluster-loader check pass but candidates check deny, retry isn’t needed
3. when two checks deny, retry isn’t needed

from incubator-uniffle.

I got your point. If you raise a pr, I'm glad to review.@zuston WDYT? @jerqi

from incubator-uniffle.

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?

from incubator-uniffle.

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).

from incubator-uniffle.

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.

from incubator-uniffle.

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

from incubator-uniffle.

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 AccessClusterloadChecker, and the issue #127 I proposed is mainly adapted to this checker, when you use this checker, you can choose to retry.

from incubator-uniffle.

But when having multiple checkers, and the apps are in candidates list, for these apps, it need retry.

from incubator-uniffle.

So do we need this feature ? cc @jerqi

from incubator-uniffle.

So do we need this feature ? cc @jerqi

I don't think we need so complex retry mechanism ...

from incubator-uniffle.

OK. Close it.

from incubator-uniffle.