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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions src/fs/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::time::SystemTime;
use bytes::Bytes;
use bytestring::ByteString;
use fuser::{FileAttr, FileType};
use tikv_client::{Transaction, TransactionClient, TransactionOptions};
use tikv_client::{Backoff, RetryOptions, Transaction, TransactionClient, TransactionOptions};
use tracing::{debug, instrument, trace};

use super::block::empty_block;
Expand All @@ -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 •
edited
Loading

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.

pub const OPTIMISTIC_BACKOFF: Backoff = Backoff::no_jitter_backoff(2, 500, 1000);

pub struct Txn {
txn: Transaction,
block_size: u64,
Expand Down Expand Up @@ -52,10 +57,16 @@ impl Txn {
max_size: Option<u64>,
max_name_len: u32,
) -> Result<Self> {
let options = TransactionOptions::new_optimistic().use_async_commit();
let options = options.retry_options(RetryOptions {
region_backoff: DEFAULT_REGION_BACKOFF,
lock_backoff: OPTIMISTIC_BACKOFF,
});
let txn: Transaction = client
.begin_with_options(options)
.await?;
Ok(Txn {
txn: client
.begin_with_options(TransactionOptions::new_optimistic().use_async_commit())
.await?,
txn,
block_size,
max_blocks: max_size.map(|size| size / block_size),
max_name_len,
Expand Down
Loading