diff --git a/cmd/bb_noop_worker/BUILD.bazel b/cmd/bb_noop_worker/BUILD.bazel index c680bf1f..966f7def 100644 --- a/cmd/bb_noop_worker/BUILD.bazel +++ b/cmd/bb_noop_worker/BUILD.bazel @@ -9,7 +9,7 @@ go_library( deps = [ "//pkg/blobstore", "//pkg/builder", - "//pkg/filesystem", + "//pkg/filesystem/pool", "//pkg/proto/configuration/bb_noop_worker", "//pkg/proto/remoteworker", "@com_github_buildbarn_bb_storage//pkg/blobstore/configuration", diff --git a/cmd/bb_noop_worker/main.go b/cmd/bb_noop_worker/main.go index ef6bef3f..2a331a9c 100644 --- a/cmd/bb_noop_worker/main.go +++ b/cmd/bb_noop_worker/main.go @@ -7,7 +7,7 @@ import ( re_blobstore "github.com/buildbarn/bb-remote-execution/pkg/blobstore" "github.com/buildbarn/bb-remote-execution/pkg/builder" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/configuration/bb_noop_worker" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" blobstore_configuration "github.com/buildbarn/bb-storage/pkg/blobstore/configuration" @@ -76,7 +76,7 @@ func main() { contentAddressableStorage, int(configuration.MaximumMessageSizeBytes), browserURL), - re_filesystem.EmptyFilePool, + pool.EmptyFilePool, clock.SystemClock, configuration.WorkerId, instanceNamePrefix, diff --git a/cmd/bb_worker/BUILD.bazel b/cmd/bb_worker/BUILD.bazel index ba6bba7b..0b635b54 100644 --- a/cmd/bb_worker/BUILD.bazel +++ b/cmd/bb_worker/BUILD.bazel @@ -16,7 +16,7 @@ go_library( "//pkg/cas", "//pkg/cleaner", "//pkg/clock", - "//pkg/filesystem", + "//pkg/filesystem/pool", "//pkg/filesystem/virtual", "//pkg/filesystem/virtual/configuration", "//pkg/proto/completedactionlogger", diff --git a/cmd/bb_worker/main.go b/cmd/bb_worker/main.go index 40754bb5..5e663147 100644 --- a/cmd/bb_worker/main.go +++ b/cmd/bb_worker/main.go @@ -18,7 +18,7 @@ import ( "github.com/buildbarn/bb-remote-execution/pkg/cas" "github.com/buildbarn/bb-remote-execution/pkg/cleaner" re_clock "github.com/buildbarn/bb-remote-execution/pkg/clock" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/virtual" virtual_configuration "github.com/buildbarn/bb-remote-execution/pkg/filesystem/virtual/configuration" cal_proto "github.com/buildbarn/bb-remote-execution/pkg/proto/completedactionlogger" @@ -76,7 +76,7 @@ func main() { // currently only used by the virtual file system to store // output files of build actions. Going forward, this may be // used to store core dumps generated by build actions as well. - filePool, err := re_filesystem.NewFilePoolFromConfiguration(configuration.FilePool) + filePool, err := pool.NewFilePoolFromConfiguration(configuration.FilePool) if err != nil { return util.StatusWrap(err, "Failed to create file pool") } @@ -218,7 +218,7 @@ func main() { virtualBuildDirectory = virtual.NewInMemoryPrepopulatedDirectory( virtual.NewHandleAllocatingFileAllocator( virtual.NewPoolBackedFileAllocator( - re_filesystem.EmptyFilePool, + pool.EmptyFilePool, util.DefaultErrorLogger), handleAllocator), symlinkFactory, @@ -466,7 +466,7 @@ func main() { buildClient := builder.NewBuildClient( schedulerClient, buildExecutor, - re_filesystem.NewQuotaEnforcingFilePool( + pool.NewQuotaEnforcingFilePool( filePool, runnerConfiguration.MaximumFilePoolFileCount, runnerConfiguration.MaximumFilePoolSizeBytes), diff --git a/internal/mock/BUILD.bazel b/internal/mock/BUILD.bazel index 3692a636..bf2061c7 100644 --- a/internal/mock/BUILD.bazel +++ b/internal/mock/BUILD.bazel @@ -172,7 +172,6 @@ gomock( out = "filesystem_re.go", interfaces = [ "DirectoryOpener", - "FilePool", "SectorAllocator", ], library = "//pkg/filesystem", @@ -181,6 +180,18 @@ gomock( package = "mock", ) +gomock( + name = "filesystem_filepool", + out = "filesystem_filepool.go", + interfaces = [ + "FilePool", + ], + library = "//pkg/filesystem/pool", + mockgen_model_library = "@org_uber_go_mock//mockgen/model", + mockgen_tool = "@org_uber_go_mock//mockgen", + package = "mock", +) + gomock( name = "filesystem_virtual", out = "filesystem_virtual.go", @@ -411,6 +422,7 @@ go_library( ":completedactionlogger.go", ":filesystem.go", ":filesystem_access.go", + ":filesystem_filepool.go", ":filesystem_re.go", ":filesystem_virtual.go", ":grpc_go.go", @@ -445,6 +457,7 @@ go_library( "//pkg/cleaner", "//pkg/filesystem", "//pkg/filesystem/access", + "//pkg/filesystem/pool", "//pkg/filesystem/virtual", "//pkg/proto/bazeloutputservice", "//pkg/proto/buildqueuestate", diff --git a/pkg/builder/BUILD.bazel b/pkg/builder/BUILD.bazel index a81db94a..a7de4111 100644 --- a/pkg/builder/BUILD.bazel +++ b/pkg/builder/BUILD.bazel @@ -36,8 +36,8 @@ go_library( "//pkg/cas", "//pkg/cleaner", "//pkg/clock", - "//pkg/filesystem", "//pkg/filesystem/access", + "//pkg/filesystem/pool", "//pkg/filesystem/virtual", "//pkg/proto/cas", "//pkg/proto/completedactionlogger", @@ -104,8 +104,8 @@ go_test( "//internal/mock", "//pkg/cleaner", "//pkg/clock", - "//pkg/filesystem", "//pkg/filesystem/access", + "//pkg/filesystem/pool", "//pkg/proto/cas", "//pkg/proto/completedactionlogger", "//pkg/proto/remoteworker", diff --git a/pkg/builder/build_client.go b/pkg/builder/build_client.go index 6d50a20f..2abf781d 100644 --- a/pkg/builder/build_client.go +++ b/pkg/builder/build_client.go @@ -6,7 +6,7 @@ import ( "time" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/clock" "github.com/buildbarn/bb-storage/pkg/digest" @@ -28,7 +28,7 @@ type BuildClient struct { // Constant fields. scheduler remoteworker.OperationQueueClient buildExecutor BuildExecutor - filePool filesystem.FilePool + filePool pool.FilePool clock clock.Clock instanceNamePrefix digest.InstanceName instanceNamePatcher digest.InstanceNamePatcher @@ -45,7 +45,7 @@ type BuildClient struct { // NewBuildClient creates a new BuildClient instance that is set to the // initial state (i.e., being idle). -func NewBuildClient(scheduler remoteworker.OperationQueueClient, buildExecutor BuildExecutor, filePool filesystem.FilePool, clock clock.Clock, workerID map[string]string, instanceNamePrefix digest.InstanceName, platform *remoteexecution.Platform, sizeClass uint32) *BuildClient { +func NewBuildClient(scheduler remoteworker.OperationQueueClient, buildExecutor BuildExecutor, filePool pool.FilePool, clock clock.Clock, workerID map[string]string, instanceNamePrefix digest.InstanceName, platform *remoteexecution.Platform, sizeClass uint32) *BuildClient { return &BuildClient{ scheduler: scheduler, buildExecutor: buildExecutor, diff --git a/pkg/builder/build_directory.go b/pkg/builder/build_directory.go index b92bbc5f..5156cbe4 100644 --- a/pkg/builder/build_directory.go +++ b/pkg/builder/build_directory.go @@ -4,8 +4,8 @@ import ( "context" "os" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-storage/pkg/digest" "github.com/buildbarn/bb-storage/pkg/filesystem" "github.com/buildbarn/bb-storage/pkg/filesystem/path" @@ -36,7 +36,7 @@ type BuildDirectory interface { // errors. Implementations of BuildDirectory are free to let // this be a no-op, with the disadvantage that they cannot apply // resource limits or provide rich I/O error messages. - InstallHooks(filePool re_filesystem.FilePool, errorLogger util.ErrorLogger) + InstallHooks(filePool pool.FilePool, errorLogger util.ErrorLogger) // Recursively merges the contents of a Directory stored in the // Content Addressable Storage into a local directory. If this diff --git a/pkg/builder/build_executor.go b/pkg/builder/build_executor.go index 68879138..5e079541 100644 --- a/pkg/builder/build_executor.go +++ b/pkg/builder/build_executor.go @@ -4,8 +4,8 @@ import ( "context" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/digest" @@ -67,5 +67,5 @@ func GetResultAndGRPCCodeFromExecuteResponse(response *remoteexecution.ExecuteRe // requests and yield an execute response. type BuildExecutor interface { CheckReadiness(ctx context.Context) error - Execute(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse + Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse } diff --git a/pkg/builder/caching_build_executor.go b/pkg/builder/caching_build_executor.go index 29942bbe..0c989fe0 100644 --- a/pkg/builder/caching_build_executor.go +++ b/pkg/builder/caching_build_executor.go @@ -5,8 +5,8 @@ import ( "net/url" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" cas_proto "github.com/buildbarn/bb-remote-execution/pkg/proto/cas" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" re_util "github.com/buildbarn/bb-remote-execution/pkg/util" @@ -42,7 +42,7 @@ func NewCachingBuildExecutor(base BuildExecutor, contentAddressableStorage, acti } } -func (be *cachingBuildExecutor) Execute(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *cachingBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { response := be.BuildExecutor.Execute(ctx, filePool, monitor, digestFunction, request, executionStateUpdates) if actionDigest, err := digestFunction.NewDigestFromProto(request.ActionDigest); err != nil { attachErrorToExecuteResponse(response, util.StatusWrap(err, "Failed to extract digest for action")) diff --git a/pkg/builder/completed_action_logging_build_executor.go b/pkg/builder/completed_action_logging_build_executor.go index 76e0072a..daf16047 100644 --- a/pkg/builder/completed_action_logging_build_executor.go +++ b/pkg/builder/completed_action_logging_build_executor.go @@ -4,8 +4,8 @@ import ( "context" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" cas_proto "github.com/buildbarn/bb-remote-execution/pkg/proto/cas" cal_proto "github.com/buildbarn/bb-remote-execution/pkg/proto/completedactionlogger" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" @@ -34,7 +34,7 @@ func NewCompletedActionLoggingBuildExecutor(base BuildExecutor, uuidGenerator ut } } -func (be *completedActionLoggingBuildExecutor) Execute(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *completedActionLoggingBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { response := be.BuildExecutor.Execute(ctx, filePool, monitor, digestFunction, request, executionStateUpdates) completedAction := &cal_proto.CompletedAction{ diff --git a/pkg/builder/cost_computing_build_executor.go b/pkg/builder/cost_computing_build_executor.go index 3ea8d993..6c44c9fb 100644 --- a/pkg/builder/cost_computing_build_executor.go +++ b/pkg/builder/cost_computing_build_executor.go @@ -5,8 +5,8 @@ import ( remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-remote-execution/pkg/proto/resourceusage" "github.com/buildbarn/bb-storage/pkg/digest" @@ -31,7 +31,7 @@ func NewCostComputingBuildExecutor(base BuildExecutor, expensesPerSecond map[str } } -func (be *costComputingBuildExecutor) Execute(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *costComputingBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { response := be.BuildExecutor.Execute(ctx, filePool, monitor, digestFunction, request, executionStateUpdates) totalTime := response.Result.ExecutionMetadata.WorkerCompletedTimestamp.AsTime().Sub(response.Result.ExecutionMetadata.WorkerStartTimestamp.AsTime()).Seconds() diff --git a/pkg/builder/file_pool_stats_build_executor.go b/pkg/builder/file_pool_stats_build_executor.go index b7888bbc..fd9a9d17 100644 --- a/pkg/builder/file_pool_stats_build_executor.go +++ b/pkg/builder/file_pool_stats_build_executor.go @@ -5,8 +5,8 @@ import ( "sync" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-remote-execution/pkg/proto/resourceusage" "github.com/buildbarn/bb-storage/pkg/digest" @@ -30,7 +30,7 @@ func NewFilePoolStatsBuildExecutor(buildExecutor BuildExecutor) BuildExecutor { } } -func (be *filePoolStatsBuildExecutor) Execute(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *filePoolStatsBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { fp := statsCollectingFilePool{base: filePool} response := be.BuildExecutor.Execute(ctx, &fp, monitor, digestFunction, request, executionStateUpdates) @@ -49,7 +49,7 @@ func (be *filePoolStatsBuildExecutor) Execute(ctx context.Context, filePool re_f // statsCollectingFilePool is a decorator for FilePool that measures the // number of files created and the number of operations performed. type statsCollectingFilePool struct { - base re_filesystem.FilePool + base pool.FilePool lock sync.Mutex stats resourceusage.FilePoolResourceUsage diff --git a/pkg/builder/file_pool_stats_build_executor_test.go b/pkg/builder/file_pool_stats_build_executor_test.go index b269c82a..f64e2470 100644 --- a/pkg/builder/file_pool_stats_build_executor_test.go +++ b/pkg/builder/file_pool_stats_build_executor_test.go @@ -8,8 +8,8 @@ import ( remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" "github.com/buildbarn/bb-remote-execution/internal/mock" "github.com/buildbarn/bb-remote-execution/pkg/builder" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-remote-execution/pkg/proto/resourceusage" "github.com/buildbarn/bb-storage/pkg/digest" @@ -43,7 +43,7 @@ func TestFilePoolStatsBuildExecutorExample(t *testing.T) { monitor, digest.MustNewFunction("hello", remoteexecution.DigestFunction_MD5), request, - gomock.Any()).DoAndReturn(func(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { + gomock.Any()).DoAndReturn(func(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { f, err := filePool.NewFile() require.NoError(t, err) require.NoError(t, f.Truncate(5)) @@ -70,12 +70,29 @@ func TestFilePoolStatsBuildExecutorExample(t *testing.T) { } }) + // Mock all the file operations performed in the execution request. + filePool := mock.NewMockFilePool(ctrl) + file1 := mock.NewMockFileReadWriter(ctrl) + file2 := mock.NewMockFileReadWriter(ctrl) + + filePool.EXPECT().NewFile().Return(file1, nil) + file1.EXPECT().Truncate(int64(5)).Return(nil) + file1.EXPECT().Close().Return(nil) + filePool.EXPECT().NewFile().Return(file2, nil) + file2.EXPECT().WriteAt([]byte("Hello"), int64(100)).Return(5, nil) + file2.EXPECT().ReadAt(gomock.Any(), int64(98)).DoAndReturn(func(p []byte, offset int64) (int, error) { + copy(p, []byte("\x00\x00Hello\x00\x00\x00")) + return 7, io.EOF + }) + file2.EXPECT().Truncate(int64(42)).Return(nil) + file2.EXPECT().Close().Return(nil) + // Perform the execution request. executionStateUpdates := make(chan *remoteworker.CurrentState_Executing, 3) buildExecutor := builder.NewFilePoolStatsBuildExecutor(baseBuildExecutor) executeResponse := buildExecutor.Execute( ctx, - filesystem.InMemoryFilePool, + filePool, monitor, digest.MustNewFunction("hello", remoteexecution.DigestFunction_MD5), request, diff --git a/pkg/builder/local_build_executor.go b/pkg/builder/local_build_executor.go index a2a1c77d..a468dc61 100644 --- a/pkg/builder/local_build_executor.go +++ b/pkg/builder/local_build_executor.go @@ -8,8 +8,8 @@ import ( remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" re_clock "github.com/buildbarn/bb-remote-execution/pkg/clock" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" runner_pb "github.com/buildbarn/bb-remote-execution/pkg/proto/runner" "github.com/buildbarn/bb-storage/pkg/blobstore" @@ -127,7 +127,7 @@ func (be *localBuildExecutor) CheckReadiness(ctx context.Context) error { return err } -func (be *localBuildExecutor) Execute(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *localBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { // Timeout handling. response := NewDefaultExecuteResponse(request) action := request.Action diff --git a/pkg/builder/logging_build_executor.go b/pkg/builder/logging_build_executor.go index e1917ae0..16fc1f47 100644 --- a/pkg/builder/logging_build_executor.go +++ b/pkg/builder/logging_build_executor.go @@ -6,8 +6,8 @@ import ( "net/url" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" re_util "github.com/buildbarn/bb-remote-execution/pkg/util" "github.com/buildbarn/bb-storage/pkg/digest" @@ -31,7 +31,7 @@ func NewLoggingBuildExecutor(base BuildExecutor, browserURL *url.URL) BuildExecu } } -func (be *loggingBuildExecutor) Execute(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *loggingBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { // Print URL to bb_browser prior to execution. if actionDigest, err := digestFunction.NewDigestFromProto(request.ActionDigest); err == nil { log.Printf("Action: %s with timeout %s", re_util.GetBrowserURL(be.browserURL, "action", actionDigest), request.Action.GetTimeout().AsDuration()) diff --git a/pkg/builder/metrics_build_executor.go b/pkg/builder/metrics_build_executor.go index 169bd572..8c5069f3 100644 --- a/pkg/builder/metrics_build_executor.go +++ b/pkg/builder/metrics_build_executor.go @@ -5,8 +5,8 @@ import ( "sync" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-remote-execution/pkg/proto/resourceusage" "github.com/buildbarn/bb-storage/pkg/digest" @@ -292,7 +292,7 @@ func observeTimestampDelta(histogram prometheus.Observer, pbStart, pbCompleted * histogram.Observe(pbCompleted.AsTime().Sub(pbStart.AsTime()).Seconds()) } -func (be *metricsBuildExecutor) Execute(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *metricsBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { response := be.BuildExecutor.Execute(ctx, filePool, monitor, digestFunction, request, executionStateUpdates) result, grpcCode := GetResultAndGRPCCodeFromExecuteResponse(response) diff --git a/pkg/builder/naive_build_directory.go b/pkg/builder/naive_build_directory.go index d36ac325..2da7f8cb 100644 --- a/pkg/builder/naive_build_directory.go +++ b/pkg/builder/naive_build_directory.go @@ -6,8 +6,8 @@ import ( "math" "github.com/buildbarn/bb-remote-execution/pkg/cas" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-storage/pkg/blobstore" "github.com/buildbarn/bb-storage/pkg/blobstore/buffer" "github.com/buildbarn/bb-storage/pkg/digest" @@ -74,7 +74,7 @@ func (d *naiveBuildDirectory) EnterUploadableDirectory(name path.Component) (Upl return d.EnterBuildDirectory(name) } -func (d *naiveBuildDirectory) InstallHooks(filePool re_filesystem.FilePool, errorLogger util.ErrorLogger) { +func (d *naiveBuildDirectory) InstallHooks(filePool pool.FilePool, errorLogger util.ErrorLogger) { // Simply ignore the provided hooks, as POSIX offers no way to // install them. This means no quota enforcement and detection // of I/O errors is performed. diff --git a/pkg/builder/noop_build_executor.go b/pkg/builder/noop_build_executor.go index 0a276443..6eeddd1c 100644 --- a/pkg/builder/noop_build_executor.go +++ b/pkg/builder/noop_build_executor.go @@ -7,8 +7,8 @@ import ( "text/template" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" re_util "github.com/buildbarn/bb-remote-execution/pkg/util" "github.com/buildbarn/bb-storage/pkg/blobstore" @@ -48,7 +48,7 @@ var defaultNoopErrorMessageTemplate = template.Must( template.New("NoopBuildExecutor"). Parse("Action has been uploaded, but will not be executed. Action details: {{ .ActionURL }}")) -func (be *noopBuildExecutor) Execute(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *noopBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { // Obtain action digest, which can be embedded in the error message. response := NewDefaultExecuteResponse(request) actionDigest, err := digestFunction.NewDigestFromProto(request.ActionDigest) diff --git a/pkg/builder/prefetching_build_executor.go b/pkg/builder/prefetching_build_executor.go index 0df2ce9a..f93378f6 100644 --- a/pkg/builder/prefetching_build_executor.go +++ b/pkg/builder/prefetching_build_executor.go @@ -7,8 +7,8 @@ import ( remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" "github.com/buildbarn/bb-remote-execution/pkg/cas" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/blobstore" "github.com/buildbarn/bb-storage/pkg/blobstore/buffer" @@ -76,7 +76,7 @@ func (be *prefetchingBuildExecutor) computeProfile(monitor *access.BloomFilterCo } } -func (be *prefetchingBuildExecutor) Execute(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *prefetchingBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { // Obtain the reduced Action digest, which is needed to read // from, and write to the File System Access Cache (FSAC). action := request.Action diff --git a/pkg/builder/prefetching_build_executor_test.go b/pkg/builder/prefetching_build_executor_test.go index 57aa4bbb..4a9e9ce7 100644 --- a/pkg/builder/prefetching_build_executor_test.go +++ b/pkg/builder/prefetching_build_executor_test.go @@ -7,8 +7,8 @@ import ( remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" "github.com/buildbarn/bb-remote-execution/internal/mock" "github.com/buildbarn/bb-remote-execution/pkg/builder" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-remote-execution/pkg/proto/resourceusage" "github.com/buildbarn/bb-storage/pkg/blobstore/buffer" @@ -182,7 +182,7 @@ func TestPrefetchingBuildExecutor(t *testing.T) { digestFunction, testutil.EqProto(t, exampleRequest), executionStateUpdates, - ).DoAndReturn(func(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { + ).DoAndReturn(func(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { <-ctx.Done() require.Equal(t, context.Canceled, ctx.Err()) return &remoteexecution.ExecuteResponse{ @@ -224,7 +224,7 @@ func TestPrefetchingBuildExecutor(t *testing.T) { digestFunction, testutil.EqProto(t, exampleRequest), executionStateUpdates, - ).DoAndReturn(func(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { + ).DoAndReturn(func(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { <-ctx.Done() require.Equal(t, context.Canceled, ctx.Err()) return &remoteexecution.ExecuteResponse{ @@ -271,7 +271,7 @@ func TestPrefetchingBuildExecutor(t *testing.T) { digestFunction, testutil.EqProto(t, exampleRequest), executionStateUpdates, - ).DoAndReturn(func(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { + ).DoAndReturn(func(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { <-ctx.Done() require.Equal(t, context.Canceled, ctx.Err()) return &remoteexecution.ExecuteResponse{ @@ -416,7 +416,7 @@ func TestPrefetchingBuildExecutor(t *testing.T) { digestFunction, testutil.EqProto(t, exampleRequest), executionStateUpdates, - ).DoAndReturn(func(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { + ).DoAndReturn(func(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { monitor.ReadDirectory() return &remoteexecution.ExecuteResponse{ Result: &remoteexecution.ActionResult{ diff --git a/pkg/builder/storage_flushing_build_executor.go b/pkg/builder/storage_flushing_build_executor.go index fd48c26b..3265c0ad 100644 --- a/pkg/builder/storage_flushing_build_executor.go +++ b/pkg/builder/storage_flushing_build_executor.go @@ -4,8 +4,8 @@ import ( "context" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/digest" ) @@ -31,7 +31,7 @@ func NewStorageFlushingBuildExecutor(base BuildExecutor, flush StorageFlusher) B } } -func (be *storageFlushingBuildExecutor) Execute(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *storageFlushingBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { response := be.BuildExecutor.Execute(ctx, filePool, monitor, digestFunction, request, executionStateUpdates) if err := be.flush(ctx); err != nil { attachErrorToExecuteResponse(response, err) diff --git a/pkg/builder/test_infrastructure_failure_detecting_build_executor.go b/pkg/builder/test_infrastructure_failure_detecting_build_executor.go index 0fe76003..23ed82e0 100644 --- a/pkg/builder/test_infrastructure_failure_detecting_build_executor.go +++ b/pkg/builder/test_infrastructure_failure_detecting_build_executor.go @@ -7,8 +7,8 @@ import ( "sync/atomic" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/digest" "github.com/buildbarn/bb-storage/pkg/program" @@ -105,7 +105,7 @@ func (be *testInfrastructureFailureDetectingBuildExecutor) CheckReadiness(ctx co }) } -func (be *testInfrastructureFailureDetectingBuildExecutor) Execute(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *testInfrastructureFailureDetectingBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { if err := be.shutdownState.isShutDown(); err != nil { response := NewDefaultExecuteResponse(request) attachErrorToExecuteResponse(response, err) diff --git a/pkg/builder/timestamped_build_executor.go b/pkg/builder/timestamped_build_executor.go index d4248583..4ebdc92c 100644 --- a/pkg/builder/timestamped_build_executor.go +++ b/pkg/builder/timestamped_build_executor.go @@ -4,8 +4,8 @@ import ( "context" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/clock" "github.com/buildbarn/bb-storage/pkg/digest" @@ -38,7 +38,7 @@ func (be *timestampedBuildExecutor) getCurrentTime() *timestamppb.Timestamp { return timestamppb.New(be.clock.Now()) } -func (be *timestampedBuildExecutor) Execute(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *timestampedBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { // Initial metadata, using the current time as the start timestamp. metadata := remoteexecution.ExecutedActionMetadata{ Worker: be.workerName, diff --git a/pkg/builder/timestamped_build_executor_test.go b/pkg/builder/timestamped_build_executor_test.go index c60457f2..5160b497 100644 --- a/pkg/builder/timestamped_build_executor_test.go +++ b/pkg/builder/timestamped_build_executor_test.go @@ -8,8 +8,8 @@ import ( remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" "github.com/buildbarn/bb-remote-execution/internal/mock" "github.com/buildbarn/bb-remote-execution/pkg/builder" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/digest" "github.com/buildbarn/bb-storage/pkg/testutil" @@ -69,7 +69,7 @@ func TestTimestampedBuildExecutorExample(t *testing.T) { monitor, digest.MustNewFunction("main", remoteexecution.DigestFunction_MD5), request, - gomock.Any()).DoAndReturn(func(ctx context.Context, filePool filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { + gomock.Any()).DoAndReturn(func(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { clock.EXPECT().Now().Return(time.Unix(1001, 0)) executionStateUpdates <- updateFetchingInputs clock.EXPECT().Now().Return(time.Unix(1002, 0)) diff --git a/pkg/builder/tracing_build_executor.go b/pkg/builder/tracing_build_executor.go index 4dab29e2..7b0a7a59 100644 --- a/pkg/builder/tracing_build_executor.go +++ b/pkg/builder/tracing_build_executor.go @@ -4,8 +4,8 @@ import ( "context" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/digest" @@ -29,7 +29,7 @@ func NewTracingBuildExecutor(buildExecutor BuildExecutor, tracerProvider trace.T } } -func (be *tracingBuildExecutor) Execute(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { +func (be *tracingBuildExecutor) Execute(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { actionDigest := request.ActionDigest action := request.Action ctxWithTracing, span := be.tracer.Start(ctx, "BuildExecutor.Execute", trace.WithAttributes( diff --git a/pkg/builder/tracing_build_executor_test.go b/pkg/builder/tracing_build_executor_test.go index 897be759..7d60e27b 100644 --- a/pkg/builder/tracing_build_executor_test.go +++ b/pkg/builder/tracing_build_executor_test.go @@ -7,8 +7,8 @@ import ( remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" "github.com/buildbarn/bb-remote-execution/internal/mock" "github.com/buildbarn/bb-remote-execution/pkg/builder" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/remoteworker" "github.com/buildbarn/bb-storage/pkg/digest" "github.com/buildbarn/bb-storage/pkg/testutil" @@ -65,7 +65,7 @@ func TestTracingBuildExecutor(t *testing.T) { monitor := mock.NewMockUnreadDirectoryMonitor(ctrl) digestFunction := digest.MustNewFunction("hello", remoteexecution.DigestFunction_SHA256) baseBuildExecutor.EXPECT().Execute(ctxWithTracing, filePool, monitor, digestFunction, testutil.EqProto(t, request), gomock.Any()).DoAndReturn( - func(ctx context.Context, filePool re_filesystem.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { + func(ctx context.Context, filePool pool.FilePool, monitor access.UnreadDirectoryMonitor, digestFunction digest.Function, request *remoteworker.DesiredState_Executing, executionStateUpdates chan<- *remoteworker.CurrentState_Executing) *remoteexecution.ExecuteResponse { executionStateUpdates <- fetchingInputs executionStateUpdates <- running executionStateUpdates <- uploadingOutputs diff --git a/pkg/builder/virtual_build_directory.go b/pkg/builder/virtual_build_directory.go index d1e0a90e..f31e84b2 100644 --- a/pkg/builder/virtual_build_directory.go +++ b/pkg/builder/virtual_build_directory.go @@ -6,8 +6,8 @@ import ( "syscall" "github.com/buildbarn/bb-remote-execution/pkg/cas" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/access" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/filesystem/virtual" "github.com/buildbarn/bb-storage/pkg/blobstore" "github.com/buildbarn/bb-storage/pkg/digest" @@ -78,7 +78,7 @@ func (d *virtualBuildDirectory) EnterUploadableDirectory(name path.Component) (U return d.EnterBuildDirectory(name) } -func (d *virtualBuildDirectory) InstallHooks(filePool re_filesystem.FilePool, errorLogger util.ErrorLogger) { +func (d *virtualBuildDirectory) InstallHooks(filePool pool.FilePool, errorLogger util.ErrorLogger) { d.PrepopulatedDirectory.InstallHooks( virtual.NewHandleAllocatingFileAllocator( virtual.NewPoolBackedFileAllocator(filePool, errorLogger), diff --git a/pkg/filesystem/BUILD.bazel b/pkg/filesystem/BUILD.bazel index b41e3db9..4e37db74 100644 --- a/pkg/filesystem/BUILD.bazel +++ b/pkg/filesystem/BUILD.bazel @@ -4,26 +4,15 @@ go_library( name = "filesystem", srcs = [ "bitmap_sector_allocator.go", - "block_device_backed_file_pool.go", - "configuration.go", - "directory_backed_file_pool.go", - "empty_file_pool.go", - "file_pool.go", - "in_memory_file_pool.go", "lazy_directory.go", - "metrics_file_pool.go", - "quota_enforcing_file_pool.go", "sector_allocator.go", ], importpath = "github.com/buildbarn/bb-remote-execution/pkg/filesystem", visibility = ["//visibility:public"], deps = [ - "//pkg/proto/configuration/filesystem", - "@com_github_buildbarn_bb_storage//pkg/blockdevice", "@com_github_buildbarn_bb_storage//pkg/filesystem", "@com_github_buildbarn_bb_storage//pkg/filesystem/path", "@com_github_buildbarn_bb_storage//pkg/util", - "@com_github_prometheus_client_golang//prometheus", "@org_golang_google_grpc//codes", "@org_golang_google_grpc//status", ], @@ -33,12 +22,7 @@ go_test( name = "filesystem_test", srcs = [ "bitmap_sector_allocator_test.go", - "block_device_backed_file_pool_test.go", - "directory_backed_file_pool_test.go", - "empty_file_pool_test.go", - "in_memory_file_pool_test.go", "lazy_directory_test.go", - "quota_enforcing_file_pool_test.go", ], deps = [ ":filesystem", diff --git a/pkg/filesystem/directory_backed_file_pool.go b/pkg/filesystem/directory_backed_file_pool.go deleted file mode 100644 index 2cbe1276..00000000 --- a/pkg/filesystem/directory_backed_file_pool.go +++ /dev/null @@ -1,106 +0,0 @@ -package filesystem - -import ( - "io" - "os" - "strconv" - "sync/atomic" - - "github.com/buildbarn/bb-storage/pkg/filesystem" - "github.com/buildbarn/bb-storage/pkg/filesystem/path" -) - -type directoryBackedFilePool struct { - directory filesystem.Directory - - nextID atomic.Uint64 -} - -// NewDirectoryBackedFilePool creates a FilePool that stores all data -// written to files into a single directory on disk. Files stored in the -// underlying directory are simply identified by an incrementing number. -// -// As many files may exist at a given point in time, this implementation -// does not keep any backing files open. This would exhaust the worker's -// file descriptor table. Files are opened on demand. -// -// TODO: Maybe use an eviction.Set to keep a small number of files open? -func NewDirectoryBackedFilePool(directory filesystem.Directory) FilePool { - return &directoryBackedFilePool{ - directory: directory, - } -} - -func (fp *directoryBackedFilePool) NewFile() (filesystem.FileReadWriter, error) { - return &lazyOpeningSelfDeletingFile{ - directory: fp.directory, - name: path.MustNewComponent(strconv.FormatUint(fp.nextID.Add(1), 10)), - }, nil -} - -// lazyOpeningSelfDeletingFile is a file descriptor that forwards -// operations to a file that is opened on demand. Upon closure, the -// underlying file is unlinked. -type lazyOpeningSelfDeletingFile struct { - directory filesystem.Directory - name path.Component -} - -func (f *lazyOpeningSelfDeletingFile) Close() error { - if err := f.directory.Remove(f.name); err != nil && !os.IsNotExist(err) { - return err - } - return nil -} - -func (f *lazyOpeningSelfDeletingFile) GetNextRegionOffset(off int64, regionType filesystem.RegionType) (int64, error) { - fh, err := f.directory.OpenRead(f.name) - if os.IsNotExist(err) { - // Empty file that doesn't explicitly exist in the - // backing store yet. Treat it as if it's a zero-length - // file. - return 0, io.EOF - } else if err != nil { - return 0, err - } - defer fh.Close() - return fh.GetNextRegionOffset(off, regionType) -} - -func (f *lazyOpeningSelfDeletingFile) ReadAt(p []byte, off int64) (int, error) { - fh, err := f.directory.OpenRead(f.name) - if os.IsNotExist(err) { - // Empty file that doesn't explicitly exist in the - // backing store yet. Treat it as if it's a zero-length - // file. - return 0, io.EOF - } else if err != nil { - return 0, err - } - defer fh.Close() - return fh.ReadAt(p, off) -} - -func (f *lazyOpeningSelfDeletingFile) Sync() error { - // Because FilePool does not provide any persistency, there is - // no need to synchronize any data. - return nil -} - -func (f *lazyOpeningSelfDeletingFile) Truncate(size int64) error { - fh, err := f.directory.OpenWrite(f.name, filesystem.CreateReuse(0o600)) - if err != nil { - return err - } - defer fh.Close() - return fh.Truncate(size) -} - -func (f *lazyOpeningSelfDeletingFile) WriteAt(p []byte, off int64) (int, error) { - fh, err := f.directory.OpenWrite(f.name, filesystem.CreateReuse(0o600)) - if err != nil { - return 0, err - } - defer fh.Close() - return fh.WriteAt(p, off) -} diff --git a/pkg/filesystem/directory_backed_file_pool_test.go b/pkg/filesystem/directory_backed_file_pool_test.go deleted file mode 100644 index 8142ec81..00000000 --- a/pkg/filesystem/directory_backed_file_pool_test.go +++ /dev/null @@ -1,96 +0,0 @@ -package filesystem_test - -import ( - "io" - "syscall" - "testing" - - "github.com/buildbarn/bb-remote-execution/internal/mock" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" - "github.com/buildbarn/bb-storage/pkg/filesystem" - "github.com/buildbarn/bb-storage/pkg/filesystem/path" - "github.com/stretchr/testify/require" - - "go.uber.org/mock/gomock" -) - -func TestDirectoryBackedFilePool(t *testing.T) { - ctrl := gomock.NewController(t) - - directory := mock.NewMockDirectory(ctrl) - fp := re_filesystem.NewDirectoryBackedFilePool(directory) - - t.Run("EmptyFile", func(t *testing.T) { - f, err := fp.NewFile() - require.NoError(t, err) - - // Underlying file should not yet exist. This should be - // interpreted as if the file is empty. - directory.EXPECT().OpenRead(path.MustNewComponent("1")).Return(nil, syscall.ENOENT) - var p [10]byte - n, err := f.ReadAt(p[:], 0) - require.Equal(t, 0, n) - require.Equal(t, io.EOF, err) - - // GetNextRegionOffset() should behave similarly. - directory.EXPECT().OpenRead(path.MustNewComponent("1")).Return(nil, syscall.ENOENT) - _, err = f.GetNextRegionOffset(0, filesystem.Data) - require.Equal(t, io.EOF, err) - - directory.EXPECT().OpenRead(path.MustNewComponent("1")).Return(nil, syscall.ENOENT) - _, err = f.GetNextRegionOffset(0, filesystem.Hole) - require.Equal(t, io.EOF, err) - - directory.EXPECT().Remove(path.MustNewComponent("1")).Return(syscall.ENOENT) - require.NoError(t, f.Close()) - }) - - t.Run("NonEmptyFile", func(t *testing.T) { - f, err := fp.NewFile() - require.NoError(t, err) - - // Write a piece of text into the file. - fileWriter := mock.NewMockFileWriter(ctrl) - directory.EXPECT().OpenWrite(path.MustNewComponent("2"), filesystem.CreateReuse(0o600)).Return(fileWriter, nil) - fileWriter.EXPECT().WriteAt([]byte("Hello, world"), int64(123)).Return(12, nil) - fileWriter.EXPECT().Close() - n, err := f.WriteAt([]byte("Hello, world"), 123) - require.Equal(t, 12, n) - require.NoError(t, err) - - // Truncate a part of it. - fileWriter = mock.NewMockFileWriter(ctrl) - directory.EXPECT().OpenWrite(path.MustNewComponent("2"), filesystem.CreateReuse(0o600)).Return(fileWriter, nil) - fileWriter.EXPECT().Truncate(int64(128)) - fileWriter.EXPECT().Close() - require.NoError(t, f.Truncate(128)) - - // Read back the end of the file. - fileReader := mock.NewMockFileReader(ctrl) - directory.EXPECT().OpenRead(path.MustNewComponent("2")).Return(fileReader, nil) - fileReader.EXPECT().ReadAt(gomock.Any(), int64(120)).DoAndReturn( - func(p []byte, off int64) (int, error) { - require.Len(t, p, 10) - copy(p, "\x00\x00\x00Hello") - return 8, io.EOF - }) - fileReader.EXPECT().Close() - var p [10]byte - n, err = f.ReadAt(p[:], 120) - require.Equal(t, 8, n) - require.Equal(t, io.EOF, err) - require.Equal(t, []byte("\x00\x00\x00Hello"), p[:8]) - - // Calls for GetNextRegionOffset() should be forwarded. - fileReader = mock.NewMockFileReader(ctrl) - directory.EXPECT().OpenRead(path.MustNewComponent("2")).Return(fileReader, nil) - fileReader.EXPECT().GetNextRegionOffset(int64(0), filesystem.Hole).Return(int64(123), nil) - fileReader.EXPECT().Close() - off, err := f.GetNextRegionOffset(0, filesystem.Hole) - require.NoError(t, err) - require.Equal(t, int64(123), off) - - directory.EXPECT().Remove(path.MustNewComponent("2")).Return(nil) - require.NoError(t, f.Close()) - }) -} diff --git a/pkg/filesystem/in_memory_file_pool.go b/pkg/filesystem/in_memory_file_pool.go deleted file mode 100644 index 04f0c528..00000000 --- a/pkg/filesystem/in_memory_file_pool.go +++ /dev/null @@ -1,81 +0,0 @@ -package filesystem - -import ( - "io" - - "github.com/buildbarn/bb-storage/pkg/filesystem" -) - -type inMemoryFilePool struct{} - -func (fp inMemoryFilePool) NewFile() (filesystem.FileReadWriter, error) { - return &inMemoryFile{}, nil -} - -type inMemoryFile struct { - data []byte -} - -func (f *inMemoryFile) Close() error { - f.data = nil - return nil -} - -func (f *inMemoryFile) GetNextRegionOffset(off int64, regionType filesystem.RegionType) (int64, error) { - // Files are stored in a byte slice contiguously, so there is no - // sparseness. - if off >= int64(len(f.data)) { - return 0, io.EOF - } - switch regionType { - case filesystem.Data: - return off, nil - case filesystem.Hole: - return int64(len(f.data)), nil - default: - panic("Unknown region type") - } -} - -func (f *inMemoryFile) ReadAt(p []byte, off int64) (int, error) { - if int(off) >= len(f.data) { - return 0, io.EOF - } - if n := copy(p, f.data[off:]); n < len(p) { - return n, io.EOF - } - return len(p), nil -} - -func (f *inMemoryFile) Sync() error { - // Because FilePool does not provide any persistency, there is - // no need to synchronize any data. - return nil -} - -func (f *inMemoryFile) Truncate(size int64) error { - if len(f.data) >= int(size) { - // Truncate the file. - f.data = f.data[:size] - } else { - // Grow the file. - f.data = append(f.data, make([]byte, int(size)-len(f.data))...) - } - return nil -} - -func (f *inMemoryFile) WriteAt(p []byte, off int64) (int, error) { - // Zero-sized writes should not cause the file to grow. - if len(p) == 0 { - return 0, nil - } - - if size := int(off) + len(p); len(f.data) < size { - // Grow the file. - f.data = append(f.data, make([]byte, size-len(f.data))...) - } - return copy(f.data[off:], p), nil -} - -// InMemoryFilePool is a FilePool that stores all data in memory. -var InMemoryFilePool FilePool = inMemoryFilePool{} diff --git a/pkg/filesystem/in_memory_file_pool_test.go b/pkg/filesystem/in_memory_file_pool_test.go deleted file mode 100644 index 079b4c91..00000000 --- a/pkg/filesystem/in_memory_file_pool_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package filesystem_test - -import ( - "io" - "testing" - - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" - "github.com/stretchr/testify/require" -) - -func TestInMemoryFilePool(t *testing.T) { - fp := filesystem.InMemoryFilePool - - t.Run("EmptyFile", func(t *testing.T) { - f, err := fp.NewFile() - require.NoError(t, err) - - var p [10]byte - n, err := f.ReadAt(p[:], 0) - require.Equal(t, 0, n) - require.Equal(t, io.EOF, err) - - require.NoError(t, f.Close()) - }) - - t.Run("NonEmptyFile", func(t *testing.T) { - f, err := fp.NewFile() - require.NoError(t, err) - - // Write a piece of text into the file. - n, err := f.WriteAt([]byte("Hello, world"), 123) - require.Equal(t, 12, n) - require.NoError(t, err) - - // Truncate a part of it. - require.NoError(t, f.Truncate(128)) - - // Read back the end of the file. - var p [10]byte - n, err = f.ReadAt(p[:], 120) - require.Equal(t, 8, n) - require.Equal(t, io.EOF, err) - require.Equal(t, []byte("\x00\x00\x00Hello"), p[:8]) - - require.NoError(t, f.Close()) - }) - - t.Run("ZeroSizedWrite", func(t *testing.T) { - f, err := fp.NewFile() - require.NoError(t, err) - - // A zero-sized write should not cause the file to - // actually grow. The read should still return EOF. - n, err := f.WriteAt(nil, 123) - require.Equal(t, 0, n) - require.NoError(t, err) - - var p [10]byte - n, err = f.ReadAt(p[:], 0) - require.Equal(t, 0, n) - require.Equal(t, io.EOF, err) - - require.NoError(t, f.Close()) - }) -} diff --git a/pkg/filesystem/pool/BUILD.bazel b/pkg/filesystem/pool/BUILD.bazel new file mode 100644 index 00000000..d15f1d50 --- /dev/null +++ b/pkg/filesystem/pool/BUILD.bazel @@ -0,0 +1,44 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "pool", + srcs = [ + "block_device_backed_file_pool.go", + "configuration.go", + "empty_file_pool.go", + "file_pool.go", + "metrics_file_pool.go", + "quota_enforcing_file_pool.go", + ], + importpath = "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool", + visibility = ["//visibility:public"], + deps = [ + "//pkg/filesystem", + "//pkg/proto/configuration/filesystem", + "@com_github_buildbarn_bb_storage//pkg/blockdevice", + "@com_github_buildbarn_bb_storage//pkg/filesystem", + "@com_github_buildbarn_bb_storage//pkg/util", + "@com_github_prometheus_client_golang//prometheus", + "@org_golang_google_grpc//codes", + "@org_golang_google_grpc//status", + ], +) + +go_test( + name = "pool_test", + srcs = [ + "block_device_backed_file_pool_test.go", + "empty_file_pool_test.go", + "quota_enforcing_file_pool_test.go", + ], + deps = [ + ":pool", + "//internal/mock", + "@com_github_buildbarn_bb_storage//pkg/filesystem", + "@com_github_buildbarn_bb_storage//pkg/testutil", + "@com_github_stretchr_testify//require", + "@org_golang_google_grpc//codes", + "@org_golang_google_grpc//status", + "@org_uber_go_mock//gomock", + ], +) diff --git a/pkg/filesystem/block_device_backed_file_pool.go b/pkg/filesystem/pool/block_device_backed_file_pool.go similarity index 98% rename from pkg/filesystem/block_device_backed_file_pool.go rename to pkg/filesystem/pool/block_device_backed_file_pool.go index f3e92b1c..5f50c9e0 100644 --- a/pkg/filesystem/block_device_backed_file_pool.go +++ b/pkg/filesystem/pool/block_device_backed_file_pool.go @@ -1,9 +1,10 @@ -package filesystem +package pool import ( "fmt" "io" + re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" "github.com/buildbarn/bb-storage/pkg/blockdevice" "github.com/buildbarn/bb-storage/pkg/filesystem" @@ -13,7 +14,7 @@ import ( type blockDeviceBackedFilePool struct { blockDevice blockdevice.BlockDevice - sectorAllocator SectorAllocator + sectorAllocator re_filesystem.SectorAllocator sectorSizeBytes int zeroSector []byte } @@ -23,7 +24,7 @@ type blockDeviceBackedFilePool struct { // device tends to be faster than using a directory on a file system, // for the reason that no metadata (e.g., a directory hierarchy and // inode attributes) needs to be stored. -func NewBlockDeviceBackedFilePool(blockDevice blockdevice.BlockDevice, sectorAllocator SectorAllocator, sectorSizeBytes int) FilePool { +func NewBlockDeviceBackedFilePool(blockDevice blockdevice.BlockDevice, sectorAllocator re_filesystem.SectorAllocator, sectorSizeBytes int) FilePool { return &blockDeviceBackedFilePool{ blockDevice: blockDevice, sectorAllocator: sectorAllocator, diff --git a/pkg/filesystem/block_device_backed_file_pool_test.go b/pkg/filesystem/pool/block_device_backed_file_pool_test.go similarity index 98% rename from pkg/filesystem/block_device_backed_file_pool_test.go rename to pkg/filesystem/pool/block_device_backed_file_pool_test.go index dac1bc36..ac96c0ea 100644 --- a/pkg/filesystem/block_device_backed_file_pool_test.go +++ b/pkg/filesystem/pool/block_device_backed_file_pool_test.go @@ -1,4 +1,4 @@ -package filesystem_test +package pool_test import ( "io" @@ -6,7 +6,7 @@ import ( "testing" "github.com/buildbarn/bb-remote-execution/internal/mock" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-storage/pkg/filesystem" "github.com/buildbarn/bb-storage/pkg/testutil" "github.com/stretchr/testify/require" @@ -22,7 +22,7 @@ func TestBlockDeviceBackedFilePool(t *testing.T) { blockDevice := mock.NewMockBlockDevice(ctrl) sectorAllocator := mock.NewMockSectorAllocator(ctrl) - pool := re_filesystem.NewBlockDeviceBackedFilePool(blockDevice, sectorAllocator, 16) + pool := pool.NewBlockDeviceBackedFilePool(blockDevice, sectorAllocator, 16) t.Run("ReadEmptyFile", func(t *testing.T) { // Test that reads on an empty file work as expected. diff --git a/pkg/filesystem/configuration.go b/pkg/filesystem/pool/configuration.go similarity index 66% rename from pkg/filesystem/configuration.go rename to pkg/filesystem/pool/configuration.go index 94850c56..b5655d38 100644 --- a/pkg/filesystem/configuration.go +++ b/pkg/filesystem/pool/configuration.go @@ -1,12 +1,11 @@ -package filesystem +package pool import ( "math" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem" pb "github.com/buildbarn/bb-remote-execution/pkg/proto/configuration/filesystem" "github.com/buildbarn/bb-storage/pkg/blockdevice" - "github.com/buildbarn/bb-storage/pkg/filesystem" - "github.com/buildbarn/bb-storage/pkg/filesystem/path" "github.com/buildbarn/bb-storage/pkg/util" "google.golang.org/grpc/codes" @@ -25,18 +24,6 @@ func NewFilePoolFromConfiguration(configuration *pb.FilePoolConfiguration) (File var filePool FilePool switch backend := configuration.Backend.(type) { - case *pb.FilePoolConfiguration_InMemory: - filePool = InMemoryFilePool - case *pb.FilePoolConfiguration_DirectoryPath: - directory, err := filesystem.NewLocalDirectory(path.LocalFormat.NewParser(backend.DirectoryPath)) - if err != nil { - return nil, util.StatusWrapf(err, "Failed to open directory %#v", backend.DirectoryPath) - } - if err := directory.RemoveAllChildren(); err != nil { - directory.Close() - return nil, util.StatusWrapf(err, "Failed to empty out directory %#v", backend.DirectoryPath) - } - filePool = NewDirectoryBackedFilePool(directory) case *pb.FilePoolConfiguration_BlockDevice: blockDevice, sectorSizeBytes, sectorCount, err := blockdevice.NewBlockDeviceFromConfiguration(backend.BlockDevice, true) if err != nil { @@ -47,7 +34,7 @@ func NewFilePoolFromConfiguration(configuration *pb.FilePoolConfiguration) (File } filePool = NewBlockDeviceBackedFilePool( blockDevice, - NewBitmapSectorAllocator(uint32(sectorCount)), + filesystem.NewBitmapSectorAllocator(uint32(sectorCount)), sectorSizeBytes) default: return nil, status.Error(codes.InvalidArgument, "Configuration did not contain a supported file pool backend") diff --git a/pkg/filesystem/empty_file_pool.go b/pkg/filesystem/pool/empty_file_pool.go similarity index 96% rename from pkg/filesystem/empty_file_pool.go rename to pkg/filesystem/pool/empty_file_pool.go index 1e60af32..cc788f54 100644 --- a/pkg/filesystem/empty_file_pool.go +++ b/pkg/filesystem/pool/empty_file_pool.go @@ -1,4 +1,4 @@ -package filesystem +package pool import ( "github.com/buildbarn/bb-storage/pkg/filesystem" diff --git a/pkg/filesystem/empty_file_pool_test.go b/pkg/filesystem/pool/empty_file_pool_test.go similarity index 67% rename from pkg/filesystem/empty_file_pool_test.go rename to pkg/filesystem/pool/empty_file_pool_test.go index e5696fa3..3faca7bc 100644 --- a/pkg/filesystem/empty_file_pool_test.go +++ b/pkg/filesystem/pool/empty_file_pool_test.go @@ -1,9 +1,9 @@ -package filesystem_test +package pool_test import ( "testing" - "github.com/buildbarn/bb-remote-execution/pkg/filesystem" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" @@ -11,6 +11,6 @@ import ( ) func TestEmptyFilePool(t *testing.T) { - _, err := filesystem.EmptyFilePool.NewFile() + _, err := pool.EmptyFilePool.NewFile() require.Equal(t, err, status.Error(codes.ResourceExhausted, "Cannot create file in empty file pool")) } diff --git a/pkg/filesystem/file_pool.go b/pkg/filesystem/pool/file_pool.go similarity index 95% rename from pkg/filesystem/file_pool.go rename to pkg/filesystem/pool/file_pool.go index 1dc91206..77c3a67b 100644 --- a/pkg/filesystem/file_pool.go +++ b/pkg/filesystem/pool/file_pool.go @@ -1,4 +1,4 @@ -package filesystem +package pool import ( "github.com/buildbarn/bb-storage/pkg/filesystem" diff --git a/pkg/filesystem/metrics_file_pool.go b/pkg/filesystem/pool/metrics_file_pool.go similarity index 98% rename from pkg/filesystem/metrics_file_pool.go rename to pkg/filesystem/pool/metrics_file_pool.go index 7938cd96..ae22e8c6 100644 --- a/pkg/filesystem/metrics_file_pool.go +++ b/pkg/filesystem/pool/metrics_file_pool.go @@ -1,4 +1,4 @@ -package filesystem +package pool import ( "sync" diff --git a/pkg/filesystem/quota_enforcing_file_pool.go b/pkg/filesystem/pool/quota_enforcing_file_pool.go similarity index 99% rename from pkg/filesystem/quota_enforcing_file_pool.go rename to pkg/filesystem/pool/quota_enforcing_file_pool.go index 0b071312..95f4309a 100644 --- a/pkg/filesystem/quota_enforcing_file_pool.go +++ b/pkg/filesystem/pool/quota_enforcing_file_pool.go @@ -1,4 +1,4 @@ -package filesystem +package pool import ( "sync/atomic" diff --git a/pkg/filesystem/quota_enforcing_file_pool_test.go b/pkg/filesystem/pool/quota_enforcing_file_pool_test.go similarity index 95% rename from pkg/filesystem/quota_enforcing_file_pool_test.go rename to pkg/filesystem/pool/quota_enforcing_file_pool_test.go index d41363fa..db414b96 100644 --- a/pkg/filesystem/quota_enforcing_file_pool_test.go +++ b/pkg/filesystem/pool/quota_enforcing_file_pool_test.go @@ -1,11 +1,11 @@ -package filesystem_test +package pool_test import ( "io" "testing" "github.com/buildbarn/bb-remote-execution/internal/mock" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-storage/pkg/filesystem" "github.com/stretchr/testify/require" @@ -18,7 +18,7 @@ import ( // testRemainingQuota is a helper function for the // QuotaEnforcingFilePool tests to check that a certain amount of space // is available within the pool. -func testRemainingQuota(t *testing.T, ctrl *gomock.Controller, underlyingPool *mock.MockFilePool, pool re_filesystem.FilePool, filesRemaining int, bytesRemaining int64) { +func testRemainingQuota(t *testing.T, ctrl *gomock.Controller, underlyingPool *mock.MockFilePool, pool pool.FilePool, filesRemaining int, bytesRemaining int64) { // Check that the remaining number of files is available by // allocating all of them. underlyingFiles := make([]*mock.MockFileReadWriter, filesRemaining) @@ -57,7 +57,7 @@ func TestQuotaEnforcingFilePoolExample(t *testing.T) { // An empty pool should have the advertised amount of space available. underlyingPool := mock.NewMockFilePool(ctrl) - pool := re_filesystem.NewQuotaEnforcingFilePool(underlyingPool, 10, 1000) + pool := pool.NewQuotaEnforcingFilePool(underlyingPool, 10, 1000) testRemainingQuota(t, ctrl, underlyingPool, pool, 10, 1000) // Failure to allocate a file from the underlying pool should diff --git a/pkg/filesystem/virtual/BUILD.bazel b/pkg/filesystem/virtual/BUILD.bazel index 25a53d84..a193728f 100644 --- a/pkg/filesystem/virtual/BUILD.bazel +++ b/pkg/filesystem/virtual/BUILD.bazel @@ -44,8 +44,8 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/cas", - "//pkg/filesystem", "//pkg/filesystem/access", + "//pkg/filesystem/pool", "//pkg/proto/bazeloutputservice", "//pkg/proto/bazeloutputservice/rev2", "//pkg/proto/outputpathpersistency", diff --git a/pkg/filesystem/virtual/pool_backed_file_allocator.go b/pkg/filesystem/virtual/pool_backed_file_allocator.go index 1e2c5d00..a4a1572f 100644 --- a/pkg/filesystem/virtual/pool_backed_file_allocator.go +++ b/pkg/filesystem/virtual/pool_backed_file_allocator.go @@ -9,7 +9,7 @@ import ( "time" remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - re_filesystem "github.com/buildbarn/bb-remote-execution/pkg/filesystem" + "github.com/buildbarn/bb-remote-execution/pkg/filesystem/pool" "github.com/buildbarn/bb-remote-execution/pkg/proto/bazeloutputservice" bazeloutputservicerev2 "github.com/buildbarn/bb-remote-execution/pkg/proto/bazeloutputservice/rev2" "github.com/buildbarn/bb-storage/pkg/blobstore" @@ -45,7 +45,7 @@ var ( ) type poolBackedFileAllocator struct { - pool re_filesystem.FilePool + pool pool.FilePool errorLogger util.ErrorLogger } @@ -58,7 +58,7 @@ type poolBackedFileAllocator struct { // file descriptor count reach zero), Close() is called on the // underlying backing file descriptor. This may be used to request // deletion from underlying storage. -func NewPoolBackedFileAllocator(pool re_filesystem.FilePool, errorLogger util.ErrorLogger) FileAllocator { +func NewPoolBackedFileAllocator(pool pool.FilePool, errorLogger util.ErrorLogger) FileAllocator { poolBackedFileAllocatorPrometheusMetrics.Do(func() { prometheus.MustRegister(poolBackedFileAllocatorWritableFileUploadDelaySeconds) prometheus.MustRegister(poolBackedFileAllocatorWritableFileUploadDelayTimeouts) diff --git a/pkg/proto/configuration/filesystem/BUILD.bazel b/pkg/proto/configuration/filesystem/BUILD.bazel index eda00b89..69e6725f 100644 --- a/pkg/proto/configuration/filesystem/BUILD.bazel +++ b/pkg/proto/configuration/filesystem/BUILD.bazel @@ -6,10 +6,7 @@ proto_library( name = "filesystem_proto", srcs = ["filesystem.proto"], visibility = ["//visibility:public"], - deps = [ - "@com_github_buildbarn_bb_storage//pkg/proto/configuration/blockdevice:blockdevice_proto", - "@protobuf//:empty_proto", - ], + deps = ["@com_github_buildbarn_bb_storage//pkg/proto/configuration/blockdevice:blockdevice_proto"], ) go_proto_library( diff --git a/pkg/proto/configuration/filesystem/filesystem.pb.go b/pkg/proto/configuration/filesystem/filesystem.pb.go index 70770b9c..6bd6aded 100644 --- a/pkg/proto/configuration/filesystem/filesystem.pb.go +++ b/pkg/proto/configuration/filesystem/filesystem.pb.go @@ -10,7 +10,6 @@ import ( blockdevice "github.com/buildbarn/bb-storage/pkg/proto/configuration/blockdevice" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoimpl "google.golang.org/protobuf/runtime/protoimpl" - emptypb "google.golang.org/protobuf/types/known/emptypb" reflect "reflect" sync "sync" unsafe "unsafe" @@ -27,8 +26,6 @@ type FilePoolConfiguration struct { state protoimpl.MessageState `protogen:"open.v1"` // Types that are valid to be assigned to Backend: // - // *FilePoolConfiguration_InMemory - // *FilePoolConfiguration_DirectoryPath // *FilePoolConfiguration_BlockDevice Backend isFilePoolConfiguration_Backend `protobuf_oneof:"backend"` unknownFields protoimpl.UnknownFields @@ -72,24 +69,6 @@ func (x *FilePoolConfiguration) GetBackend() isFilePoolConfiguration_Backend { return nil } -func (x *FilePoolConfiguration) GetInMemory() *emptypb.Empty { - if x != nil { - if x, ok := x.Backend.(*FilePoolConfiguration_InMemory); ok { - return x.InMemory - } - } - return nil -} - -func (x *FilePoolConfiguration) GetDirectoryPath() string { - if x != nil { - if x, ok := x.Backend.(*FilePoolConfiguration_DirectoryPath); ok { - return x.DirectoryPath - } - } - return "" -} - func (x *FilePoolConfiguration) GetBlockDevice() *blockdevice.Configuration { if x != nil { if x, ok := x.Backend.(*FilePoolConfiguration_BlockDevice); ok { @@ -103,34 +82,20 @@ type isFilePoolConfiguration_Backend interface { isFilePoolConfiguration_Backend() } -type FilePoolConfiguration_InMemory struct { - InMemory *emptypb.Empty `protobuf:"bytes,1,opt,name=in_memory,json=inMemory,proto3,oneof"` -} - -type FilePoolConfiguration_DirectoryPath struct { - DirectoryPath string `protobuf:"bytes,2,opt,name=directory_path,json=directoryPath,proto3,oneof"` -} - type FilePoolConfiguration_BlockDevice struct { BlockDevice *blockdevice.Configuration `protobuf:"bytes,3,opt,name=block_device,json=blockDevice,proto3,oneof"` } -func (*FilePoolConfiguration_InMemory) isFilePoolConfiguration_Backend() {} - -func (*FilePoolConfiguration_DirectoryPath) isFilePoolConfiguration_Backend() {} - func (*FilePoolConfiguration_BlockDevice) isFilePoolConfiguration_Backend() {} var File_pkg_proto_configuration_filesystem_filesystem_proto protoreflect.FileDescriptor const file_pkg_proto_configuration_filesystem_filesystem_proto_rawDesc = "" + "\n" + - "3pkg/proto/configuration/filesystem/filesystem.proto\x12\"buildbarn.configuration.filesystem\x1a\x1bgoogle/protobuf/empty.proto\x1a5pkg/proto/configuration/blockdevice/blockdevice.proto\"\xdb\x01\n" + - "\x15FilePoolConfiguration\x125\n" + - "\tin_memory\x18\x01 \x01(\v2\x16.google.protobuf.EmptyH\x00R\binMemory\x12'\n" + - "\x0edirectory_path\x18\x02 \x01(\tH\x00R\rdirectoryPath\x12W\n" + + "3pkg/proto/configuration/filesystem/filesystem.proto\x12\"buildbarn.configuration.filesystem\x1a5pkg/proto/configuration/blockdevice/blockdevice.proto\"\x87\x01\n" + + "\x15FilePoolConfiguration\x12W\n" + "\fblock_device\x18\x03 \x01(\v22.buildbarn.configuration.blockdevice.ConfigurationH\x00R\vblockDeviceB\t\n" + - "\abackendBMZKgithub.com/buildbarn/bb-remote-execution/pkg/proto/configuration/filesystemb\x06proto3" + "\abackendJ\x04\b\x01\x10\x02J\x04\b\x02\x10\x03BMZKgithub.com/buildbarn/bb-remote-execution/pkg/proto/configuration/filesystemb\x06proto3" var ( file_pkg_proto_configuration_filesystem_filesystem_proto_rawDescOnce sync.Once @@ -147,17 +112,15 @@ func file_pkg_proto_configuration_filesystem_filesystem_proto_rawDescGZIP() []by var file_pkg_proto_configuration_filesystem_filesystem_proto_msgTypes = make([]protoimpl.MessageInfo, 1) var file_pkg_proto_configuration_filesystem_filesystem_proto_goTypes = []any{ (*FilePoolConfiguration)(nil), // 0: buildbarn.configuration.filesystem.FilePoolConfiguration - (*emptypb.Empty)(nil), // 1: google.protobuf.Empty - (*blockdevice.Configuration)(nil), // 2: buildbarn.configuration.blockdevice.Configuration + (*blockdevice.Configuration)(nil), // 1: buildbarn.configuration.blockdevice.Configuration } var file_pkg_proto_configuration_filesystem_filesystem_proto_depIdxs = []int32{ - 1, // 0: buildbarn.configuration.filesystem.FilePoolConfiguration.in_memory:type_name -> google.protobuf.Empty - 2, // 1: buildbarn.configuration.filesystem.FilePoolConfiguration.block_device:type_name -> buildbarn.configuration.blockdevice.Configuration - 2, // [2:2] is the sub-list for method output_type - 2, // [2:2] is the sub-list for method input_type - 2, // [2:2] is the sub-list for extension type_name - 2, // [2:2] is the sub-list for extension extendee - 0, // [0:2] is the sub-list for field type_name + 1, // 0: buildbarn.configuration.filesystem.FilePoolConfiguration.block_device:type_name -> buildbarn.configuration.blockdevice.Configuration + 1, // [1:1] is the sub-list for method output_type + 1, // [1:1] is the sub-list for method input_type + 1, // [1:1] is the sub-list for extension type_name + 1, // [1:1] is the sub-list for extension extendee + 0, // [0:1] is the sub-list for field type_name } func init() { file_pkg_proto_configuration_filesystem_filesystem_proto_init() } @@ -166,8 +129,6 @@ func file_pkg_proto_configuration_filesystem_filesystem_proto_init() { return } file_pkg_proto_configuration_filesystem_filesystem_proto_msgTypes[0].OneofWrappers = []any{ - (*FilePoolConfiguration_InMemory)(nil), - (*FilePoolConfiguration_DirectoryPath)(nil), (*FilePoolConfiguration_BlockDevice)(nil), } type x struct{} diff --git a/pkg/proto/configuration/filesystem/filesystem.proto b/pkg/proto/configuration/filesystem/filesystem.proto index e16c4534..3841970b 100644 --- a/pkg/proto/configuration/filesystem/filesystem.proto +++ b/pkg/proto/configuration/filesystem/filesystem.proto @@ -2,20 +2,23 @@ syntax = "proto3"; package buildbarn.configuration.filesystem; -import "google/protobuf/empty.proto"; import "pkg/proto/configuration/blockdevice/blockdevice.proto"; option go_package = "github.com/buildbarn/bb-remote-execution/pkg/proto/configuration/filesystem"; message FilePoolConfiguration { - oneof backend { - // Store all temporary files in memory. - google.protobuf.Empty in_memory = 1; + // Was 'in_memory'. This filepool configuration did not have support + // for sparse files and performed poorly when memory constrained. It + // has been removed. + reserved 1; - // Store all temporary files in a single directory on a file system. - // This option denotes the path of this directory. - string directory_path = 2; + // Was 'directory_path'. Write operations were implemented by opening + // the file, writing to it, and closing it. Reading was implemented + // similarly. This led to significant performance overhead. Use + // 'block_device' backed by a single file instead. + reserved 2; + oneof backend { // Store all temporary files in a single file on a file system or on // a raw block device. buildbarn.configuration.blockdevice.Configuration block_device = 3;