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

storage: possibly incorrect use of Bsize instead of Frsize in sysutil.StatFS #61182

Open
stevendanna opened this issue Feb 26, 2021 · 2 comments
Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@stevendanna
Copy link
Collaborator

stevendanna commented Feb 26, 2021

Describe the problem

In sysutil.StatFS we return the Bsize field as the "BlockSize":

BlockSize: int64(fs.Bsize),

free := fs.AvailBlocks * fs.BlockSize

Unfortunately, on Linux, I believe that the frsize field should be preferred over bsize when calculating total sizes from statfs output. While counterintuitive based on their plain-language descriptions, the authors of GNU coreutils and gnulib seem to agree. The issue here: cockroachdb/pebble#1072 provides an example of the potential problem.

Jira issue: CRDB-3055

@stevendanna stevendanna added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 26, 2021
@jbowens
Copy link
Collaborator

jbowens commented Jun 9, 2021

This is also potentially a problem with the capacity calculations that power the capacity timeseries metric (and rebalancing decisions). Currently, we use the github.com/elastic/gosigar module for this purpose:

if err := fileSystemUsage.Get(dir); err != nil {

https://github.com/elastic/gosigar/blob/v0.14.1/sigar_unix.go#L23

@jbowens jbowens added the A-storage Relating to our storage engine (Pebble) on-disk storage. label Jun 9, 2021
@stevendanna stevendanna changed the title Possibly incorrect use of Bsize instead of Frsize in sysutil.StatFS storage: possibly incorrect use of Bsize instead of Frsize in sysutil.StatFS Sep 1, 2021
@github-actions
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

No branches or pull requests

2 participants