Repository navigation
fix: stop re-listing on every lookup of a name past the last known key #54
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
44c9c1d
fix: make the slurp gap cache work, and remember when a listing ran o…
efirs 79a5cd3
fix: actually emit loaded ranges in the inode debug dump
efirs fc3e70c
test: lookups past the last listed key must not list again
efirs 9c099a1
fix: an explicit refresh must forget the loaded ranges too
efirs 8b6d9ce
fix: drop loaded ranges at the invalidation entry points, not inside …
efirs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| // Copyright 2026 Tigris Data, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package core | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "sync/atomic" | ||
| "syscall" | ||
| "time" | ||
|
|
||
| . "gopkg.in/check.v1" | ||
| ) | ||
|
|
||
| // A writer that keeps creating files whose names sort after every key on the | ||
| // server used to make every lookup of the next name list from the server: the | ||
| // listing found nothing past the name, and that was never remembered. | ||
| func (s *GoofysTest) TestLookupPastLastKeyDoesNotRelist(t *C) { | ||
| s.fs.flags.StatCacheTTL = time.Minute | ||
| root := s.getRoot(t) | ||
|
|
||
| s.setupBlobs(s.cloud, t, map[string]*string{ | ||
| "gapdir/file-0001": nil, | ||
| "gapdir/file-0002": nil, | ||
| "gapdir/file-0003": nil, | ||
| }) | ||
|
|
||
| var lists, heads int64 | ||
| s3 := root.fs.getCloud() | ||
| cloud := NewTestBackend(&TestBackend{StorageBackend: s3}) | ||
| cloud.ListBlobsFunc = func(p *ListBlobsInput) (*ListBlobsOutput, error) { | ||
| atomic.AddInt64(&lists, 1) | ||
| return s3.ListBlobs(p) | ||
| } | ||
| cloud.HeadBlobFunc = func(p *HeadBlobInput) (*HeadBlobOutput, error) { | ||
| atomic.AddInt64(&heads, 1) | ||
| return s3.HeadBlob(p) | ||
| } | ||
| root.fs.setCloud(cloud) | ||
|
|
||
| // Force the directory to be consulted from the server. | ||
| root.mu.Lock() | ||
| root.dir.DirTime = time.Time{} | ||
| root.mu.Unlock() | ||
| dir, err := root.LookUpCached("gapdir") | ||
| t.Assert(err, IsNil) | ||
| dir.mu.Lock() | ||
| dir.dir.DirTime = time.Time{} | ||
| dir.mu.Unlock() | ||
|
|
||
| // The first unknown name past the last key may have to list once. | ||
| _, err = dir.LookUpCached("file-0100") | ||
| t.Assert(err, Equals, syscall.ENOENT) | ||
| listsAfterFirst := atomic.LoadInt64(&lists) | ||
| headsAfterFirst := atomic.LoadInt64(&heads) | ||
|
|
||
| // Every later name is covered by that listing: nothing follows it. | ||
| for i := 101; i < 110; i++ { | ||
| _, err = dir.LookUpCached(fmt.Sprintf("file-%04d", i)) | ||
| t.Assert(err, Equals, syscall.ENOENT) | ||
| } | ||
| t.Assert(atomic.LoadInt64(&lists), Equals, listsAfterFirst, | ||
| Commentf("later names must not list again; lists went %d -> %d", listsAfterFirst, atomic.LoadInt64(&lists))) | ||
| t.Assert(atomic.LoadInt64(&heads), Equals, headsAfterFirst, | ||
| Commentf("later names must not HEAD; heads went %d -> %d", headsAfterFirst, atomic.LoadInt64(&heads))) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| // Copyright 2026 Tigris Data, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package core | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| "github.com/tigrisdata/tigrisfs/core/cfg" | ||
| ) | ||
|
|
||
| func cfgDefaultFlagsForTest() *cfg.FlagStorage { return cfg.DefaultFlags() } | ||
|
|
||
| func gapRanges(d *DirInodeData) [][2]string { | ||
| out := make([][2]string, 0, len(d.Gaps)) | ||
| for _, g := range d.Gaps { | ||
| out = append(out, [2]string{g.start, g.end}) | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| // The slurp cache was inert: markGapLoaded's final-length arithmetic was one | ||
| // short, so the range it inserted was truncated away every time. With Gaps | ||
| // always empty, every lookup on an expired directory went back to the server | ||
| // for a listing and a HEAD. | ||
| func TestMarkGapLoadedKeepsWhatItInserts(t *testing.T) { | ||
| old := time.Now().Add(-time.Hour) | ||
| d := &DirInodeData{} | ||
|
|
||
| d.markGapLoaded("a", "m") | ||
| require.Equal(t, [][2]string{{"a", "m"}}, gapRanges(d)) | ||
| require.True(t, d.checkGapLoaded("b", old)) | ||
| require.False(t, d.checkGapLoaded("a", old), "start is exclusive: StartAfter never returns it") | ||
| require.True(t, d.checkGapLoaded("m", old), "end is inclusive") | ||
| require.False(t, d.checkGapLoaded("z", old)) | ||
|
|
||
| d.markGapLoaded("p", "s") | ||
| require.Equal(t, [][2]string{{"a", "m"}, {"p", "s"}}, gapRanges(d), "append after") | ||
|
|
||
| d.markGapLoaded("0", "5") | ||
| require.Equal(t, [][2]string{{"0", "5"}, {"a", "m"}, {"p", "s"}}, gapRanges(d), "insert before") | ||
| } | ||
|
|
||
| func TestMarkGapLoadedOverlaps(t *testing.T) { | ||
| d := &DirInodeData{} | ||
| d.markGapLoaded("a", "m") | ||
| d.markGapLoaded("p", "s") | ||
|
|
||
| d.markGapLoaded("k", "q") | ||
| require.Equal(t, [][2]string{{"a", "k"}, {"k", "q"}, {"q", "s"}}, gapRanges(d), | ||
| "overlapping neighbours are trimmed to the parts outside the new range") | ||
|
|
||
| d.markGapLoaded("b", "r") | ||
| require.Equal(t, [][2]string{{"a", "b"}, {"b", "r"}, {"r", "s"}}, gapRanges(d), | ||
| "a new range covering several old ones replaces their interiors") | ||
|
|
||
| d.markGapLoaded("", gapEndOfListing) | ||
| require.Equal(t, [][2]string{{"", gapEndOfListing}}, gapRanges(d), "a full listing subsumes everything") | ||
| } | ||
|
|
||
| // A narrow re-list inside an open-ended range must not throw away what is | ||
| // known past its end, or the next lookup of a later name lists again. | ||
| func TestMarkGapLoadedSplitsAContainingRange(t *testing.T) { | ||
| old := time.Now().Add(-time.Hour) | ||
| d := &DirInodeData{} | ||
| d.markGapLoaded("a", gapEndOfListing) | ||
|
|
||
| d.markGapLoaded("f", "h") | ||
| require.Equal(t, [][2]string{{"a", "f"}, {"f", "h"}, {"h", gapEndOfListing}}, gapRanges(d)) | ||
| require.True(t, d.checkGapLoaded("zzz", old), "still known that nothing follows h") | ||
| } | ||
|
|
||
| // This is the reported symptom: an appending writer creates names sorting | ||
| // after every key on the server. A listing from such a name returns nothing, | ||
| // and must be remembered as "nothing after this", not as an empty range. | ||
| func TestGapRecordsEndOfListing(t *testing.T) { | ||
| old := time.Now().Add(-time.Hour) | ||
| d := &DirInodeData{} | ||
|
|
||
| // What the slurp records when the listing is not truncated. | ||
| d.markGapLoaded("dir/file-0100", gapEndOfListing) | ||
|
|
||
| for _, later := range []string{"dir/file-0101", "dir/file-0101/", "dir/file-9999", "zzz"} { | ||
| require.True(t, d.checkGapLoaded(later, old), "%q sorts after the last listed key", later) | ||
| } | ||
| require.False(t, d.checkGapLoaded("dir/file-0099", old), "earlier names were not listed") | ||
|
|
||
| // Keys are UTF-8, so the sentinel must sort after any of them. | ||
| require.True(t, d.checkGapLoaded("\U0010FFFF", old)) | ||
| require.True(t, d.checkGapLoaded("\xf4\x8f\xbf\xbf", old)) | ||
| } | ||
|
|
||
| func TestCheckGapLoadedDropsStaleRanges(t *testing.T) { | ||
| d := &DirInodeData{} | ||
| d.markGapLoaded("a", "m") | ||
| d.Gaps[0].loadTime = time.Now().Add(-2 * time.Minute) | ||
|
|
||
| require.False(t, d.checkGapLoaded("b", time.Now().Add(-time.Minute)), "older than the TTL") | ||
| require.Empty(t, d.Gaps, "a stale range is evicted on lookup") | ||
| } | ||
|
|
||
| // An explicit refresh must forget the loaded ranges too, or LookUp keeps | ||
| // serving the entries the refresh just expired straight from cache. | ||
| func TestResetDirTimeDropsLoadedRanges(t *testing.T) { | ||
| fs := &Goofys{flags: cfgDefaultFlagsForTest()} | ||
| root := NewInode(fs, nil, "") | ||
| root.ToDir() | ||
| sub := NewInode(fs, root, "sub") | ||
| sub.ToDir() | ||
| file := NewInode(fs, sub, "file") | ||
| root.dir.markGapLoaded("", gapEndOfListing) | ||
| require.NotEmpty(t, root.dir.Gaps) | ||
|
|
||
| // resetDirTimeRec calls this first; the rest of it needs a mounted fs, so | ||
| // the wiring is covered by the notify-refresh tests over FUSE. | ||
| file.dropLoadedRanges() | ||
| require.Empty(t, root.dir.Gaps, "refreshing any inode drops the root's ranges") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.