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

heavy load: increase timeouts and retries for spinning of transaction #7

Closed

Conversation

cre4ture
Copy link
Contributor

@cre4ture cre4ture commented May 22, 2024

Hello,

this was the next step (related to #5) I did for increasing robustness:
Increasing the timeouts and spinning delays.

I took as base values the ones that the tikv-client does by default and increased it according my gut-feeling.
I'm not an expert there and I also didn't do testing of other values and combinations,
therefor I do not guarantee for potential side effects.

Still I think it makes sence to have the option to tune these parameters. Which this PR allows.

Kind regards,
cre4ture

@@ -19,6 +19,11 @@ use super::mode::{as_file_kind, as_file_perm, make_mode};
use super::reply::{DirItem, StatFs};
use super::tikv_fs::{DIR_PARENT, DIR_SELF};

// default from tikv-rust-client: no_jitter_backoff(base_delay_ms: 2, max_delay_ms: 500, max_attempts: 10)
pub const DEFAULT_REGION_BACKOFF: Backoff = Backoff::no_jitter_backoff(300, 1000, 100);
// default from tikv-rust-client: no_jitter_backoff(base_delay_ms: 2, max_delay_ms: 500, max_attempts: 10)
Copy link
Owner

Choose a reason for hiding this comment

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

Please keep the comments consist with the real value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's intendend that they differ. I put them as reference to show what the defaults are.
Anyway. Meanwhile I did some further testing and it seems that I have some issues with the WIFI. When I do the same test with a cable ethernet connection to the cluster, results are much better. So I think its probably better to implement a command line option that switches two predefined set of values. E.g. one for "Low Latency Networks" and one for "Wireless/Mobile" connections.
What do you think?

Copy link
Owner

@Hexilee Hexilee Jun 18, 2024

Choose a reason for hiding this comment

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

In my opinion, I prefer to setting a low base_delay_ms as the default value, and left a command option and description to hint users to use a higher base_delay_ms in unstable network. The max_attempts can be a constant value, and the max_delay_ms may be N * base_delay_ms.

Providing two options to be suitable for different network environments is a black-box.

@cre4ture
Copy link
Contributor Author

sorry, I lost the interest on this topic for now. Maybe I come back later again. But for now I close the PR as it doesn't have a priority for me. @Hexilee Thanks for your time that you spend on giving me feedback. If you think this PR is of importance for you, please tell me, then I would try to find a bit of time to finish it.

@cre4ture cre4ture closed this Sep 10, 2024
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