[PFS-46] Fix directory creation with path range requests #8528
Conversation
src/server/pfs/server/source.go
Outdated
| s.fileIndexOpts = append(s.fileIndexOpts, index.WithRange(&index.PathRange{ | ||
| Lower: sc.pathRange.Lower, | ||
| Upper: sc.pathRange.Upper, | ||
| Upper: sc.pathRange.Upper + "_", |
There was a problem hiding this comment.
Is there any value to using "\0" here?
There was a problem hiding this comment.
It doesn't affect the results, but it is probably clearer what is being done if the null byte is used.
src/server/pfs/server/source.go
Outdated
| s.fileIndexOpts = append(s.fileIndexOpts, index.WithRange(&index.PathRange{ | ||
| Lower: sc.pathRange.Lower, | ||
| Upper: sc.pathRange.Upper, | ||
| Upper: sc.pathRange.Upper + "_", |
There was a problem hiding this comment.
If we don't explode on invalid paths, I would prefer to add a null byte here. I prefer the guarantee that there is no path between sc.pathRange.Upper and the index.PathRange upper bound. I believe the guarantee here is that there is no currently valid path between them. If we change the allowed character set, this could break.
There was a problem hiding this comment.
It wouldn't break with an allowable character set change because the upper bound is still used in the source iteration, but using the null byte is probably clearer anyways.
This PR fixes directory creation when the first file in the directory is the upper bound in a path range request. The source abstraction is responsible for emitting PFS directories / files and supports path range requests. The issue is that a path range is an inclusive, exclusive range, so a directory with its first file as the upper bound would not get generated since the file would not be emitted by the internal index iterators. To ensure that the correct directories are emitted in this case, the path range that is passed to the indexing layer is tweaked to iterate past the upper bound. The source iterator will exit early when the upper bound is reached or passed. This ensures that the correct directories will be emitted within the path range provided to the source. This PR also includes some additional testing for this.