Skip to content

Conversation

@meroton-benjamin
Copy link
Contributor

@meroton-benjamin meroton-benjamin commented Jun 7, 2025

Moves disk quota enforcement to sector allocator, this allows the
file pool to have arbitrary large sparse files without running into
quota issues.

A consequence of this is that very large sparse files are now a first
class citizen of Buildbarn. We have therefore modified the sparse files
implementation to use a struct inspired by the unix inode pointer
struct.

@meroton-benjamin meroton-benjamin force-pushed the pr/quota-enforced-sector-allocator branch from 0db9a22 to eeabec1 Compare September 6, 2025 10:44
@meroton-benjamin meroton-benjamin changed the title Quota enforce at sector allocator Quota enforcing sector allocator Sep 6, 2025
@meroton-benjamin meroton-benjamin force-pushed the pr/quota-enforced-sector-allocator branch 4 times, most recently from 4b48152 to 0556176 Compare September 6, 2025 15:54
@meroton-benjamin meroton-benjamin marked this pull request as ready for review September 6, 2025 16:25
@meroton-benjamin
Copy link
Contributor Author

Latest version of an implementation of a quota enforcing sector allocator. There are some possible performance improvements that have not been pursued:

GetNextUnmappedSector when run in a dense region iterates through all sectors until it finds an unmapped one, this is not worse than what was done before but if run on e.g. a 16TB dense file from byte zero would require 2^32 iterations until it found an unmapped sector. With some bookkeeping this could probably be reduced.

GetLogicalSize keeps track of the last mapped sector of the pointer by manipulating an internal variable. This needs to be recalculated whenever truncated to smaller than the mapped sector. With a better implementation of GetNextUnmappedSector this might be possible to do faster by alternating between GetNextUnmappedSector and GetNextMappedSector until it reaches the last mapped sector less than what was truncated to.

// returned. If there are no more mapped sectors then io.EOF is
// returned.
func (sp *SectorPointer) GetNextMappedSector(logical uint32) (uint32, error) {
from := logical
Copy link
Member

Choose a reason for hiding this comment

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

I think this code is a bit hard to read due to the excessive use of helper variables. What are your thoughts on getting rid of next and from? Can't we just write:

for {
    var list []uint32
    logical, list = sp.getNextDirectSectorList(logical)
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really find this suggestion making things any clearer, I appreciate that the why of GetNextMappedSector is not very clear unless you know how the helper getNextDirectSectorList works. I have added some documentation to getNextDirectSectorList and reworked it somewhat to get around an overflow bug. Please have a look again if you still find it too unclear.

@meroton-benjamin meroton-benjamin force-pushed the pr/quota-enforced-sector-allocator branch from 57ff47f to 97c21f2 Compare September 20, 2025 22:18
@meroton-benjamin
Copy link
Contributor Author

With the modifications after the pull request GetLogicalSize could be removed. Tests that verify functionality around the boundary conditions the has been added (and the corresponding bug they discovered has been fixed).

@EdSchouten
Copy link
Member

In #200 Jille and I added support for configuring the 'HoleSource' of a FilePool file. However, we didn't go as far that we changed BlockDeviceBackedFilePool to use an inode tree structure. It's still a list.

Benjamin, would you be interested in rebasing this change on top of what's in main right now? So:

  • Change BlockDeviceBackedFilePool to use an inode tree structure.
  • Change quota enforcement to account for sparseness.

@meroton-benjamin
Copy link
Contributor Author

Sure I'll get on that

@meroton-benjamin meroton-benjamin force-pushed the pr/quota-enforced-sector-allocator branch from 97c21f2 to 7006bdc Compare October 29, 2025 21:30
@aspect-workflows
Copy link

aspect-workflows bot commented Oct 29, 2025

Test

3 test targets passed

Targets
//pkg/builder:builder_test [k8-fastbuild]            77ms
//pkg/filesystem/pool:pool_test [k8-fastbuild]       488ms
//pkg/filesystem/virtual:virtual_test [k8-fastbuild] 137ms

Total test execution time was 702ms. 16 tests (84.2%) were fully cached saving 3s.

@meroton-benjamin
Copy link
Contributor Author

Rebased quota enforced sector allocator on top of what is in main and made the tests pass. As the rebase was pretty big it might be worth making another review pass at this.

@meroton-benjamin meroton-benjamin force-pushed the pr/quota-enforced-sector-allocator branch 4 times, most recently from 11dd8a9 to 3f4f917 Compare November 1, 2025 15:25
```
Moves disk quota enforcement to sector allocator, this allows the
file pool to have arbitrary large sparse files without running into
quota issues.

A consequence of this is that very large sparse files are now a first
class citizen of Buildbarn. We have therefore modified the sparse files
implementation to use a struct inspired by the unix inode pointer
struct.
```
@meroton-benjamin meroton-benjamin force-pushed the pr/quota-enforced-sector-allocator branch from 3f4f917 to 9bed942 Compare November 1, 2025 19:12
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.

2 participants