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

Sync XFS tuning with DirectPV #1369

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Sync XFS tuning with DirectPV #1369

wants to merge 1 commit into from

Conversation

@@ -314,8 +314,11 @@ Storage
deviceName="$(basename $i)"
echo "Modifying xfs max_retries and retry_timeout_seconds for drive $i mounted at $mountPath"
echo 0 > /sys/fs/xfs/$deviceName/error/metadata/EIO/max_retries
echo 5 > /sys/fs/xfs/$deviceName/error/metadata/EIO/retry_timeout_seconds
Copy link
Collaborator

Choose a reason for hiding this comment

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

@balamurugana any reason we shouldn't pull this over to our general guidance? e.g. something specific directpv is doing where it might cause an issue on other xfs drives?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually it feels somewhat extraneous - if max_retries is 0, then there's no point in setting retry_timeout_seconds right?

Copy link
Author

Choose a reason for hiding this comment

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

I can do a PR in directpv then.

Copy link
Member

Choose a reason for hiding this comment

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

@ravindk89 I couldn't able to understand the question. What is other xfs drives? DirectPV mounts device with those tunings automatically. Refer https://github.com/minio/directpv/blob/master/pkg/xfs/mount_linux.go#L30

These tunings were suggested by @harshavardhana

Copy link
Collaborator

Choose a reason for hiding this comment

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

My logic here is

  • we set max_retries to 0
  • We also set retry_timeout_seconds to 5
  • but the first setting means we never retry
  • So why do we need to set a retry timeout at all

redhat docs say "/sys/fs/xfs/device/error/metadata/condition/retry_timeout_seconds: the time limit in seconds after which XFS will stop retrying the operation " - unless this applies also to the original operation (e.g. return an error if we don't get success in 5 seconds).

As far as other xfs drives, the docs above would update our public-facing guidance on xfs tunings. If we are confident that these are generally 'safe', then there is no harm in adding it.

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.

3 participants