Store note on accuracy of initial disk usage calculation in diskUsage notes #33
Conversation
ghost
assigned 
README.md
Outdated
| @@ -60,6 +60,8 @@ datastore is not open), with the right disk usage value in size. I.e., in the da | |||
| 3919232394 . | |||
| $ echo -n "3919232394" > diskUsage.cache | |||
|
|
|||
| The accuracy of the initial disk usage calculation is stores in the file `diskUsage.notes`, this file is for reference only and is | |||
convert.go
Outdated
| "github.com/ipfs/go-datastore" | ||
| "github.com/ipfs/go-datastore/query" | ||
| "github.com/jbenet/go-os-rename" | ||
| "gx/ipfs/QmXRKBQA4wXP7xWbFiZsR1GP4HV6wMDQ1aWFxZZ4uBcPX9/go-datastore" |
There was a problem hiding this comment.
should not leave rewritten
flatfs_test.go
Outdated
| @@ -660,7 +669,7 @@ func testDiskUsageEstimation(dirFunc mkShardFunc, t *testing.T) { | |||
| if err != nil { | |||
| t.Fatal(err) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
I thought I did. Sorry.
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if string(d) != "initial-exact\n" { |
There was a problem hiding this comment.
Use constant instead of literal?
There was a problem hiding this comment.
It's a test, so I want to make sure it has the right value.
There was a problem hiding this comment.
But the right value is and will always be the one defined by the constant?
There was a problem hiding this comment.
Yes, unless its accidentally modified in which case this test will fail to alert you of that fact. If it was intentional, then the test case (and the README) should also be modified.
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if string(d) != "initial-approximate\n" { |
There was a problem hiding this comment.
It's a test, so I want to make sure it has the right value.
README.md
Outdated
| @@ -60,6 +60,8 @@ datastore is not open), with the right disk usage value in size. I.e., in the da | |||
| 3919232394 . | |||
| $ echo -n "3919232394" > diskUsage.cache | |||
|
|
|||
| The accuracy of the initial disk usage calculation is stores in the file `diskUsage.notes`, this file is for reference only and is | |||
| currently not used in any other way. | |||
There was a problem hiding this comment.
Let's explain what each of the three possible values mean, from a user perspective it's may not be clear what initial-timed-out actually signals.
kevina
force-pushed
the
kevina/diskusage-notes
branch
from
fa82562 to
9f8dd1b
Compare
The note is stored in the file 'diskUsage.notes'.
kevina
force-pushed
the
kevina/diskusage-notes
branch
from
9f8dd1b to
ca23b19
Compare
| if err != nil { | ||
| return err | ||
| } | ||
| if time.Now().After(deadline) { | ||
| if accuracy == timedoutA { |
There was a problem hiding this comment.
Don't we want to print warnings for any inaccuracy?
There was a problem hiding this comment.
Maybe. My code does not change the behavior of @hsanjuan code.
There was a problem hiding this comment.
Fair enough. We can always change this later.
| @@ -35,6 +35,9 @@ var ( | |||
| // DiskUsageFile is the name of the file to cache the size of the | |||
| // datastore in disk | |||
| DiskUsageFile = "diskUsage.cache" | |||
| // DiskUsageNodes stores notes about the accuracy of the size of | |||
| // the diskusage calculation | |||
| DiskUsageNotes = "diskUsage.notes" | |||
There was a problem hiding this comment.
Any reason not to put this in the same file? Or just have one JSON/CBOR file with all the information (significantly more flexible)? That would also allow it to be atomic.
This would also allow us to keep track of the disk size accuracy if we crash. We could record the fact that we're currently running while running. If we start up and see that we didn't correct the state, we can adjust the accuracy to approximate (or something else).
There was a problem hiding this comment.
I did it this way for simplicity. It has the advantage of not having to read the accuracy data once written, but I have no problem doing something more sophisticated.
There was a problem hiding this comment.
My motivation is really atomicity. It allows us to atomically store the repo size and it's accuracy (and allows us to deal with the inaccurate repo size on crash case). However, I don't really feel strongly any which way.
@Stebalien, please see #35 |
|
Closing in favor of #35. |
Closes #31