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

fix(baidu_netdisk): deplicate retry #7972

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

ljcbaby
Copy link

@ljcbaby ljcbaby commented Feb 9, 2025

不知道是不是 进过 errgroup 和 resty 之后, ctx 有问题。

出错了也还会把 threadG 剩下的线程执行完,这不符合预期。

尝试修复了一下,不知道这样对不对,麻烦大佬帮忙看一下。

@foxxorcat
Copy link
Contributor

是符合预期,假如在某一时刻同时启动了3个协程,只有当其中一个协程错误次数超过3次(retry.Attempts(3))时ctx才会取消,而在前两次错误时ctx不会取消所以会表现出遇到错误但依旧在上传后续分片,因为错误的协程还在重试。

@ljcbaby
Copy link
Author

ljcbaby commented Feb 10, 2025

是符合预期,假如在某一时刻同时启动了3个协程,只有当其中一个协程错误次数超过3次(retry.Attempts(3))时ctx才会取消,而在前两次错误时ctx不会取消所以会表现出遇到错误但依旧在上传后续分片,因为错误的协程还在重试。

我是在调最大分片数量的时候遇到的,当时并发开太高了,10之前就已经 retry 丢出 error(context deadline exceeded (Client.Timeout exceeded while awaiting headers)) 了,不是 warning 。但我看他还是照样执行了后面分片的上传,把所有分片都跑了一遍才报错。

@foxxorcat
Copy link
Contributor

写的时候没注意,retry 设了一个重试, resty 也有一个重试,导致一共重试了9次😂。
把retry.Attempts改成1就正常了

@ljcbaby
Copy link
Author

ljcbaby commented Feb 10, 2025

所以是 errgroupRestyClient 两层重试导致的?

@ljcbaby
Copy link
Author

ljcbaby commented Feb 10, 2025

写的时候没注意,retry 设了一个重试, resty 也有一个重试,导致一共重试了9次😂。 把retry.Attempts改成1就正常了

如果只是两层重试的话也只是重试次数多一点,不会出现错误没法打断后续执行的情况吧

@foxxorcat
Copy link
Contributor

你可以再试试,没有问题的。只是日志 error 错误(不是warning )要出现3次,这个日志是RestyClient 打印的,errgroup 不打印日志

@ljcbaby
Copy link
Author

ljcbaby commented Feb 10, 2025

你可以再试试,没有问题的。只是日志 error 错误(不是warning )要出现3次,这个日志是RestyClient 打印的,errgroup 不打印日志

改了回头再观察一下吧,RestyClient的日志没保存到日志文件里,stdout我没顺手保存下来

@ljcbaby ljcbaby changed the title fix(baidu_netdisk): break on slice transfer error fix(baidu_netdisk): deplicate retry Feb 10, 2025
@ljcbaby
Copy link
Author

ljcbaby commented Feb 16, 2025

@xhofe merge this?

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

Successfully merging this pull request may close these issues.

2 participants