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

When the doPushWithCallback push task fails, it no longer retries forever, but only the default number of retries #10727

Closed
Bo-Qiu opened this issue Jul 4, 2023 · 2 comments
Assignees
Labels

Comments

@Bo-Qiu
Copy link
Contributor

Bo-Qiu commented Jul 4, 2023

Is your feature request related to a problem? Please describe.

For the same pushTask, the com.alibaba.nacos.naming.push.v2.executor.PushExecutor#doPushWithCallback method will not retry forever after onFail, but will only retry the default number of times.
When the same pushTask fails and retries the default number of doPush times, the task will not be added back to the delayTaskEngine's task queue. In this way, you can avoid repeatedly pushing invalid tasks that are destined to fail, and you can even avoid the problem of server OOM exceptions caused by a large number of invalid tasks that cannot be successfully removed.

Describe the solution you'd like

DoPushWithCallback will retry after failing to push the same task. If the task fails every retry, it will retry forever. This will waste resources on invalid tasks that are bound to fail, and at the same time, when a large number of invalid tasks appear, it will cause the server to OOM.

After doPushWithCallback fails to push the same task, it will only perform the default number of retries. After the default number of times is exceeded, even if the task still fails to be pushed, it will not be added to the task queue.

Describe alternatives you've considered

After doPushWithCallback fails to push the same task, it will only perform the default number of retries. After the default number of times is exceeded, even if the task still fails to be pushed, it will not be added to the task queue.

Users can customize the number of retries. If not configured, the default value will be used. The default value is 3.

When the number of times configured by the user is -1, it is consistent with the logic of the current version. If the task push fails, it will keep retrying until the task push succeeds.

@Bo-Qiu
Copy link
Contributor Author

Bo-Qiu commented Jul 4, 2023

If this feature is necessary, I'll take it over and implement it as soon as possible, please assign it to me,thx!

Bo-Qiu added a commit to Bo-Qiu/nacos that referenced this issue Jul 4, 2023
…no longer retries forever, but only the default number of retries.
Bo-Qiu added a commit to Bo-Qiu/nacos that referenced this issue Jul 4, 2023
…no longer retries forever, but only the default number of retries.
@KomachiSion
Copy link
Collaborator

I see the issue, but the request is not reasonable.

The push check will see the subsribers and connection whether existed. So It is not push forever to client and not cause OOM.

If the connection and subsribers existed but the push continuely failed. Please make sure your subsribers is ok and network is ok first.

If there is limit of push count, the data in subsribers will different with server, which make bigger problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants