diff --git a/Makefile b/Makefile index 9afdc1d8e..a27572139 100644 --- a/Makefile +++ b/Makefile @@ -386,7 +386,7 @@ local-submitqueue-gateway-stop: ## Stop Gateway service local-init-submitqueue-schemas: ## Manually apply all database schemas @echo "Applying storage schema to mysql-app..." - @for file in submitqueue/extension/storage/mysql/schema/*.sql; do \ + @for file in submitqueue/orchestrator/extension/storage/mysql/schema/*.sql; do \ echo " - Applying $$(basename $$file)..."; \ docker exec -i $(SUBMITQUEUE_LOCAL_PROJECT)-mysql-app-1 mysql -uroot -proot submitqueue < $$file 2>&1 | grep -v "Using a password" || true; \ done diff --git a/service/submitqueue/orchestrator/server/BUILD.bazel b/service/submitqueue/orchestrator/server/BUILD.bazel index 8fa50c0db..d921c16a1 100644 --- a/service/submitqueue/orchestrator/server/BUILD.bazel +++ b/service/submitqueue/orchestrator/server/BUILD.bazel @@ -60,11 +60,11 @@ go_library( "//submitqueue/extension/speculation/scorer/heuristic:go_default_library", "//submitqueue/extension/speculation/speculator:go_default_library", "//submitqueue/extension/speculation/speculator/standard:go_default_library", - "//submitqueue/extension/storage:go_default_library", - "//submitqueue/extension/storage/mysql:go_default_library", "//submitqueue/extension/validator:go_default_library", "//submitqueue/extension/validator/fake:go_default_library", "//submitqueue/orchestrator:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mysql:go_default_library", "@com_github_go_sql_driver_mysql//:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@in_gopkg_yaml_v3//:go_default_library", @@ -125,7 +125,7 @@ go_test( "//submitqueue/extension/conflict:go_default_library", "//submitqueue/extension/speculation/scorer:go_default_library", "//submitqueue/extension/speculation/speculator:go_default_library", - "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/service/submitqueue/orchestrator/server/main.go b/service/submitqueue/orchestrator/server/main.go index ec5e21ce8..be46f7afb 100644 --- a/service/submitqueue/orchestrator/server/main.go +++ b/service/submitqueue/orchestrator/server/main.go @@ -26,6 +26,8 @@ import ( "syscall" "time" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + _ "github.com/go-sql-driver/mysql" "github.com/uber-go/tally" @@ -44,11 +46,10 @@ import ( "github.com/uber/submitqueue/platform/pipeline" servicemq "github.com/uber/submitqueue/service/messagequeue" "github.com/uber/submitqueue/submitqueue/core/changeset" - "github.com/uber/submitqueue/submitqueue/extension/storage" - mysqlstorage "github.com/uber/submitqueue/submitqueue/extension/storage/mysql" "github.com/uber/submitqueue/submitqueue/extension/validator" validatorfake "github.com/uber/submitqueue/submitqueue/extension/validator/fake" "github.com/uber/submitqueue/submitqueue/orchestrator" + mysqlstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mysql" "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/reflection" @@ -438,7 +439,7 @@ func getEnv(key, defaultVal string) string { } // storageFactory adapts the MySQL storage backend's queue binding to the -// storage.Factory seam. Routing every queue to the single shared backend is +// orchstorage.Factory seam. Routing every queue to the single shared backend is // this host's policy; a deployment that splits queues across backends swaps // this adapter for a routing one. type storageFactory struct { @@ -446,7 +447,7 @@ type storageFactory struct { } // For returns the queue-scoped store aggregate bound to the queue named in config. -func (f storageFactory) For(config storage.Config) (storage.Storage, error) { +func (f storageFactory) For(config orchstorage.Config) (orchstorage.Storage, error) { return f.backend.For(config.QueueName) } diff --git a/service/submitqueue/orchestrator/server/profiles.go b/service/submitqueue/orchestrator/server/profiles.go index 72c6a4e75..73d2ad7be 100644 --- a/service/submitqueue/orchestrator/server/profiles.go +++ b/service/submitqueue/orchestrator/server/profiles.go @@ -18,6 +18,8 @@ import ( "fmt" nethttp "net/http" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" "go.uber.org/zap" "golang.org/x/oauth2" @@ -54,7 +56,6 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/speculation/scorer/heuristic" "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator" specstandard "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator/standard" - "github.com/uber/submitqueue/submitqueue/extension/storage" ) // Profile holds the per-queue extension implementations. Grouping them per @@ -74,7 +75,7 @@ type Profile struct { // Storage resolves the queue-scoped store aggregate for this queue. Every // profile points at the shared backend by default; a deployment that // splits queues across storage backends overrides this per queue. - Storage storage.Factory + Storage orchstorage.Factory // Scorer holds this queue's ranking profile. There is no scoring stage: the // scorer feeds the queue's speculator, which ranks candidate paths by how @@ -143,10 +144,10 @@ func (p Profiles) ScorerFactory() scorer.Factory { }) } -// StorageFactory returns a storage.Factory that routes each queue to its +// StorageFactory returns a orchstorage.Factory that routes each queue to its // profile's storage backend before binding the queue-scoped store aggregate. -func (p Profiles) StorageFactory() storage.Factory { - return storageFunc(func(c storage.Config) (storage.Storage, error) { +func (p Profiles) StorageFactory() orchstorage.Factory { + return storageFunc(func(c orchstorage.Config) (orchstorage.Storage, error) { return p.For(c.QueueName).Storage.For(c) }) } @@ -169,9 +170,9 @@ type analyzerFunc func(conflict.Config) (conflict.Analyzer, error) func (f analyzerFunc) For(c conflict.Config) (conflict.Analyzer, error) { return f(c) } -type storageFunc func(storage.Config) (storage.Storage, error) +type storageFunc func(orchstorage.Config) (orchstorage.Storage, error) -func (f storageFunc) For(c storage.Config) (storage.Storage, error) { return f(c) } +func (f storageFunc) For(c orchstorage.Config) (orchstorage.Storage, error) { return f(c) } type scorerFunc func(scorer.Config) (scorer.Scorer, error) @@ -192,7 +193,7 @@ func newProfiles( logger *zap.Logger, scope tally.Scope, resolver changeset.Resolver, - stores storage.Factory, + stores orchstorage.Factory, cfg profilesConfig, ) (Profiles, error) { b := &profileBuilder{ @@ -245,7 +246,7 @@ type profileBuilder struct { logger *zap.Logger scope tally.Scope resolver changeset.Resolver - stores storage.Factory + stores orchstorage.Factory built map[string]any } diff --git a/service/submitqueue/orchestrator/server/profiles_test.go b/service/submitqueue/orchestrator/server/profiles_test.go index 05b41159b..1e83b93c0 100644 --- a/service/submitqueue/orchestrator/server/profiles_test.go +++ b/service/submitqueue/orchestrator/server/profiles_test.go @@ -19,6 +19,8 @@ import ( "errors" "testing" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -28,7 +30,6 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/conflict" "github.com/uber/submitqueue/submitqueue/extension/speculation/scorer" "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator" - "github.com/uber/submitqueue/submitqueue/extension/storage" ) // recorder captures the queue name each seam's factory was handed, so a test @@ -66,7 +67,7 @@ func profileRecording(rec *recorder) Profile { rec.analyzer = c.QueueName return nil, nil }), - Storage: storageFunc(func(c storage.Config) (storage.Storage, error) { + Storage: storageFunc(func(c orchstorage.Config) (orchstorage.Storage, error) { rec.storage = c.QueueName return nil, nil }), @@ -109,7 +110,7 @@ func TestProfilesForwardQueueNameToFactories(t *testing.T) { require.NoError(t, err) _, err = profiles.AnalyzerFactory().For(conflict.Config{QueueName: tt.queue}) require.NoError(t, err) - _, err = profiles.StorageFactory().For(storage.Config{QueueName: tt.queue}) + _, err = profiles.StorageFactory().For(orchstorage.Config{QueueName: tt.queue}) require.NoError(t, err) _, err = profiles.ScorerFactory().For(scorer.Config{QueueName: tt.queue}) require.NoError(t, err) diff --git a/submitqueue/core/batch/BUILD.bazel b/submitqueue/core/batch/BUILD.bazel index a6459d922..928e86a1a 100644 --- a/submitqueue/core/batch/BUILD.bazel +++ b/submitqueue/core/batch/BUILD.bazel @@ -12,6 +12,7 @@ go_library( deps = [ "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@org_golang_x_sync//errgroup:go_default_library", ], ) @@ -28,6 +29,7 @@ go_test( "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@org_uber_go_mock//gomock:go_default_library", diff --git a/submitqueue/core/batch/find.go b/submitqueue/core/batch/find.go index c028a96b2..8df7e6942 100644 --- a/submitqueue/core/batch/find.go +++ b/submitqueue/core/batch/find.go @@ -21,7 +21,8 @@ import ( "sort" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // FindByRequestID resolves every batch attempt associated with a request, @@ -35,7 +36,7 @@ import ( // // Unlike ListByStates, which treats a dangling membership record as store // corruption, a dangling association is an expected retry artifact. -func FindByRequestID(ctx context.Context, store storage.Storage, requestID string) ([]entity.Batch, int, error) { +func FindByRequestID(ctx context.Context, store orchstorage.Storage, requestID string) ([]entity.Batch, int, error) { associations, err := store.GetRequestBatchStore().GetByRequestID(ctx, requestID) if err != nil { return nil, 0, fmt.Errorf("failed to get batch associations for request %s: %w", requestID, err) diff --git a/submitqueue/core/batch/find_test.go b/submitqueue/core/batch/find_test.go index 4a0c52aba..cc0eec224 100644 --- a/submitqueue/core/batch/find_test.go +++ b/submitqueue/core/batch/find_test.go @@ -26,6 +26,7 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" ) const testRequestID = "monorepo/4" @@ -36,11 +37,11 @@ func association(batchID string) entity.RequestBatch { } // findStores wires a MockStorage over a batch store and a request-batch store. -func findStores(t *testing.T) (*storagemock.MockStorage, *storagemock.MockBatchStore, *storagemock.MockRequestBatchStore) { +func findStores(t *testing.T) (*orchstoragemock.MockStorage, *storagemock.MockBatchStore, *storagemock.MockRequestBatchStore) { t.Helper() ctrl := gomock.NewController(t) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockAssociationStore := storagemock.NewMockRequestBatchStore(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() diff --git a/submitqueue/core/batch/list.go b/submitqueue/core/batch/list.go index edd2cb7c7..be5232034 100644 --- a/submitqueue/core/batch/list.go +++ b/submitqueue/core/batch/list.go @@ -21,7 +21,7 @@ import ( "golang.org/x/sync/errgroup" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // hydrateConcurrency bounds the parallel per-key batch reads a single @@ -39,7 +39,7 @@ const hydrateConcurrency = 16 // A candidate ID whose batch does not exist is returned as an error rather than // skipped: batch rows are never deleted, so a dangling record means the store is // inconsistent, not that the batch concluded. -func ListByStates(ctx context.Context, store storage.Storage, states []entity.BatchState) ([]entity.Batch, error) { +func ListByStates(ctx context.Context, store orchstorage.Storage, states []entity.BatchState) ([]entity.Batch, error) { wanted := make(map[entity.BatchState]bool, len(states)) seen := make(map[string]bool) var ids []string diff --git a/submitqueue/core/batch/transition.go b/submitqueue/core/batch/transition.go index 32493e7fa..30a41b9a2 100644 --- a/submitqueue/core/batch/transition.go +++ b/submitqueue/core/batch/transition.go @@ -36,7 +36,7 @@ import ( "fmt" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // Transition moves a batch to newState: it performs the optimistic-locking CAS on @@ -52,7 +52,7 @@ import ( // applied — the CAS may have committed with the record move incomplete — and the // caller is expected to let redelivery retry; the retry's already-in-target-state // branch repairs the record via EnsureRecord. -func Transition(ctx context.Context, store storage.Storage, batch entity.Batch, newState entity.BatchState) (entity.Batch, error) { +func Transition(ctx context.Context, store orchstorage.Storage, batch entity.Batch, newState entity.BatchState) (entity.Batch, error) { oldState := batch.State newVersion := batch.Version + 1 updated := batch @@ -78,7 +78,7 @@ func Transition(ctx context.Context, store storage.Storage, batch entity.Batch, // the repair half of the transition protocol: idempotent redelivery branches that // skip the CAS because the batch is already in the target state call this instead, // covering a prior attempt that crashed between the CAS and the record move. -func EnsureRecord(ctx context.Context, store storage.Storage, batch entity.Batch) error { +func EnsureRecord(ctx context.Context, store orchstorage.Storage, batch entity.Batch) error { record := entity.QueueBatchState{Queue: batch.Queue, State: batch.State, BatchID: batch.ID} if err := store.GetQueueBatchStateStore().Put(ctx, record); err != nil { return fmt.Errorf("failed to put queue batch state record for batch %s under state %s: %w", batch.ID, batch.State, err) diff --git a/submitqueue/core/batch/transition_test.go b/submitqueue/core/batch/transition_test.go index 3bc814d42..dee1ae0a4 100644 --- a/submitqueue/core/batch/transition_test.go +++ b/submitqueue/core/batch/transition_test.go @@ -26,15 +26,16 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" ) // testStores wires a MockStorage whose batch and queue-batch-state accessors // return the two mocks the tests set expectations on. -func testStores(t *testing.T) (*storagemock.MockStorage, *storagemock.MockBatchStore, *storagemock.MockQueueBatchStateStore) { +func testStores(t *testing.T) (*orchstoragemock.MockStorage, *storagemock.MockBatchStore, *storagemock.MockQueueBatchStateStore) { t.Helper() ctrl := gomock.NewController(t) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockRecordStore := storagemock.NewMockQueueBatchStateStore(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() diff --git a/submitqueue/core/changeset/BUILD.bazel b/submitqueue/core/changeset/BUILD.bazel index 3a73067f2..86669806d 100644 --- a/submitqueue/core/changeset/BUILD.bazel +++ b/submitqueue/core/changeset/BUILD.bazel @@ -11,7 +11,7 @@ go_library( deps = [ "//platform/base/change:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", ], ) @@ -24,6 +24,7 @@ go_test( "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@org_uber_go_mock//gomock:go_default_library", diff --git a/submitqueue/core/changeset/resolver.go b/submitqueue/core/changeset/resolver.go index df09e3ea3..25a429ba0 100644 --- a/submitqueue/core/changeset/resolver.go +++ b/submitqueue/core/changeset/resolver.go @@ -20,25 +20,25 @@ import ( "github.com/uber/submitqueue/platform/base/change" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // resolver is the store-backed Resolver. It holds the storage factory and // resolves the batch's queue-scoped request and change stores per call, since // every resolution is for exactly one batch and the batch names its queue. type resolver struct { - stores storage.Factory + stores orchstorage.Factory } // New returns a Resolver backed by the given storage factory. -func New(stores storage.Factory) Resolver { +func New(stores orchstorage.Factory) Resolver { return resolver{stores: stores} } // ChangesForBatch resolves a batch's requests to their raw changes, in // batch.Contains order. func (r resolver) ChangesForBatch(ctx context.Context, batch entity.Batch) ([]change.Change, error) { - store, err := r.stores.For(storage.Config{QueueName: batch.Queue}) + store, err := r.stores.For(orchstorage.Config{QueueName: batch.Queue}) if err != nil { return nil, fmt.Errorf("failed to resolve storage for queue %q: %w", batch.Queue, err) } @@ -57,7 +57,7 @@ func (r resolver) ChangesForBatch(ctx context.Context, batch entity.Batch) ([]ch // ChangeInfo per claimed URI, owned by the requesting request, aggregated across // the whole batch. func (r resolver) DetailedForBatch(ctx context.Context, batch entity.Batch) (entity.BatchChanges, error) { - store, err := r.stores.For(storage.Config{QueueName: batch.Queue}) + store, err := r.stores.For(orchstorage.Config{QueueName: batch.Queue}) if err != nil { return entity.BatchChanges{}, fmt.Errorf("failed to resolve storage for queue %q: %w", batch.Queue, err) } diff --git a/submitqueue/core/changeset/resolver_test.go b/submitqueue/core/changeset/resolver_test.go index 67ec55893..63460020e 100644 --- a/submitqueue/core/changeset/resolver_test.go +++ b/submitqueue/core/changeset/resolver_test.go @@ -27,15 +27,16 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" ) // newTestResolver builds a Resolver over mock stores exposed through a mock // storage factory that resolves every queue to the same aggregate. func newTestResolver(ctrl *gomock.Controller, reqs storage.RequestStore, changes storage.ChangeStore) Resolver { - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(reqs).AnyTimes() store.EXPECT().GetChangeStore().Return(changes).AnyTimes() - f := storagemock.NewMockFactory(ctrl) + f := orchstoragemock.NewMockFactory(ctrl) f.EXPECT().For(gomock.Any()).Return(store, nil).AnyTimes() return New(f) } diff --git a/submitqueue/core/request/BUILD.bazel b/submitqueue/core/request/BUILD.bazel index ad473bbe7..10c793d39 100644 --- a/submitqueue/core/request/BUILD.bazel +++ b/submitqueue/core/request/BUILD.bazel @@ -18,6 +18,7 @@ go_library( "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/gateway/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", ], ) @@ -40,6 +41,7 @@ go_test( "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", "//submitqueue/gateway/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@org_uber_go_mock//gomock:go_default_library", diff --git a/submitqueue/core/request/materializer.go b/submitqueue/core/request/materializer.go index ee99dd214..2b149ca61 100644 --- a/submitqueue/core/request/materializer.go +++ b/submitqueue/core/request/materializer.go @@ -23,7 +23,7 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" basestorage "github.com/uber/submitqueue/submitqueue/extension/storage" - storage "github.com/uber/submitqueue/submitqueue/gateway/extension/storage" + gwstorage "github.com/uber/submitqueue/submitqueue/gateway/extension/storage" ) // Materializer appends request logs and projects the winning public request state. @@ -31,11 +31,11 @@ import ( // Every store it touches is queue-scoped, so each call resolves the aggregate once // from the queue carried on the log being persisted. type Materializer struct { - stores storage.Factory + stores gwstorage.Factory } // NewMaterializer creates a request read-model materializer. -func NewMaterializer(stores storage.Factory) *Materializer { +func NewMaterializer(stores gwstorage.Factory) *Materializer { return &Materializer{stores: stores} } @@ -43,7 +43,7 @@ func NewMaterializer(stores storage.Factory) *Materializer { // Projection errors are returned so queue deliveries are retried rather than silently dropping the side write. // Because the append happens first, retrying after a projection failure may retain another copy of the event in History. func (m *Materializer) PersistLog(ctx context.Context, log entity.RequestLog) error { - stores, err := m.stores.For(storage.Config{QueueName: log.Queue}) + stores, err := m.stores.For(gwstorage.Config{QueueName: log.Queue}) if err != nil { return fmt.Errorf("failed to resolve storage for queue %q: %w", log.Queue, err) } @@ -89,7 +89,7 @@ func (m *Materializer) PersistLog(ctx context.Context, log entity.RequestLog) er // repairPublicProjections activates and repairs the public query projections. // URI mappings are created before the queue summary, which acts as the marker that activation completed. -func (m *Materializer) repairPublicProjections(ctx context.Context, stores storage.Storage, authoritative entity.RequestSummary) error { +func (m *Materializer) repairPublicProjections(ctx context.Context, stores gwstorage.Storage, authoritative entity.RequestSummary) error { desired := queueSummaryFromSummary(authoritative) queueSummaries := stores.GetRequestQueueSummaryStore() for { @@ -126,7 +126,7 @@ func (m *Materializer) repairPublicProjections(ctx context.Context, stores stora } } -func (m *Materializer) createURIMappings(ctx context.Context, stores storage.Storage, summary entity.RequestSummary) error { +func (m *Materializer) createURIMappings(ctx context.Context, stores gwstorage.Storage, summary entity.RequestSummary) error { uris := stores.GetRequestURIStore() for _, changeURI := range summary.ChangeURIs { mapping := entity.RequestURI{ diff --git a/submitqueue/core/request/terminate.go b/submitqueue/core/request/terminate.go index 0ba045cad..73f559ef7 100644 --- a/submitqueue/core/request/terminate.go +++ b/submitqueue/core/request/terminate.go @@ -21,7 +21,8 @@ import ( "github.com/uber/submitqueue/platform/consumer" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // TerminationOutcome describes what TerminateRequest did to the request. @@ -91,7 +92,7 @@ type TerminationResult struct { // intrinsically retryable) so the caller's next attempt re-reads and re-evaluates. func TerminateRequest( ctx context.Context, - store storage.Storage, + store orchstorage.Storage, registry consumer.TopicRegistry, requestID string, targetState entity.RequestState, diff --git a/submitqueue/core/request/terminate_test.go b/submitqueue/core/request/terminate_test.go index 63c6bd756..4db2e4f04 100644 --- a/submitqueue/core/request/terminate_test.go +++ b/submitqueue/core/request/terminate_test.go @@ -29,6 +29,7 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" ) @@ -186,7 +187,7 @@ func TestTerminateRequest(t *testing.T) { t.Run(name, func(t *testing.T) { ctrl := gomock.NewController(t) requestStore := storagemock.NewMockRequestStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() tc.mockFunc(requestStore) diff --git a/submitqueue/extension/storage/BUILD.bazel b/submitqueue/extension/storage/BUILD.bazel index 1e879b6be..9dcf2e624 100644 --- a/submitqueue/extension/storage/BUILD.bazel +++ b/submitqueue/extension/storage/BUILD.bazel @@ -43,8 +43,8 @@ go_test( filegroup( name = "schema", srcs = [ - "//submitqueue/extension/storage/mysql/schema", "//submitqueue/gateway/extension/storage/mysql/schema", + "//submitqueue/orchestrator/extension/storage/mysql/schema", ], visibility = ["//visibility:public"], ) diff --git a/submitqueue/extension/storage/mock/BUILD.bazel b/submitqueue/extension/storage/mock/BUILD.bazel index 3f03707b6..94f252551 100644 --- a/submitqueue/extension/storage/mock/BUILD.bazel +++ b/submitqueue/extension/storage/mock/BUILD.bazel @@ -16,7 +16,6 @@ go_library( "request_summary_store_mock.go", "request_uri_store_mock.go", "speculation_path_set_store_mock.go", - "storage_mock.go", ], importpath = "github.com/uber/submitqueue/submitqueue/extension/storage/mock", visibility = ["//visibility:public"], diff --git a/submitqueue/extension/storage/storage.go b/submitqueue/extension/storage/storage.go index 5fd92be6f..f82729269 100644 --- a/submitqueue/extension/storage/storage.go +++ b/submitqueue/extension/storage/storage.go @@ -12,10 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Package storage holds SubmitQueue's persistence contracts: the store +// interfaces both services are written against, the error vocabulary their +// implementations return, and the queue config their factories resolve +// against. Which stores a service may reach is decided by its own aggregate — +// see submitqueue/gateway/extension/storage and +// submitqueue/orchestrator/extension/storage. package storage -//go:generate mockgen -source=storage.go -destination=mock/storage_mock.go -package=mock - import ( "errors" "fmt" @@ -53,45 +57,3 @@ type Config struct { // scoped to. QueueName string } - -// Factory resolves the queue-scoped Storage aggregate for a queue. Mirrors the -// extension contract: the host wiring decides which backend serves which -// queue; implementations bind the queue over their backend so a resolved -// instance can only read and write that queue's data. -type Factory interface { - // For returns the Storage aggregate bound to the queue named in config. - For(config Config) (Storage, error) -} - -// Storage aggregates the queue-scoped entity stores into a single injectable -// dependency. An instance is resolved per queue through Factory and is bound -// to that queue: entity arguments whose Queue field disagrees with the -// binding are rejected, and reads never surface another queue's records. -type Storage interface { - // GetRequestStore returns the RequestStore instance. - GetRequestStore() RequestStore - - // GetRequestBatchStore returns the RequestBatchStore instance. - GetRequestBatchStore() RequestBatchStore - - // GetChangeStore returns the ChangeStore instance. - GetChangeStore() ChangeStore - - // GetBatchStore returns the BatchStore instance. - GetBatchStore() BatchStore - - // GetBatchDependentStore returns the BatchDependentStore instance. - GetBatchDependentStore() BatchDependentStore - - // GetQueueBatchStateStore returns the QueueBatchStateStore instance. - GetQueueBatchStateStore() QueueBatchStateStore - - // GetBuildStore returns the BuildStore instance. - GetBuildStore() BuildStore - - // GetSpeculationPathSetStore returns the SpeculationPathSetStore instance. - GetSpeculationPathSetStore() SpeculationPathSetStore - - // GetPathBuildStore returns the PathBuildStore instance. - GetPathBuildStore() PathBuildStore -} diff --git a/submitqueue/orchestrator/BUILD.bazel b/submitqueue/orchestrator/BUILD.bazel index f806c7789..fff290a6b 100644 --- a/submitqueue/orchestrator/BUILD.bazel +++ b/submitqueue/orchestrator/BUILD.bazel @@ -18,7 +18,6 @@ go_library( "//submitqueue/extension/changeprovider:go_default_library", "//submitqueue/extension/conflict:go_default_library", "//submitqueue/extension/speculation/speculator:go_default_library", - "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/validator:go_default_library", "//submitqueue/orchestrator/controller:go_default_library", "//submitqueue/orchestrator/controller/batch:go_default_library", @@ -34,6 +33,7 @@ go_library( "//submitqueue/orchestrator/controller/speculate:go_default_library", "//submitqueue/orchestrator/controller/start:go_default_library", "//submitqueue/orchestrator/controller/validate:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], diff --git a/submitqueue/orchestrator/controller/batch/BUILD.bazel b/submitqueue/orchestrator/controller/batch/BUILD.bazel index ae667d36d..ad665c22c 100644 --- a/submitqueue/orchestrator/controller/batch/BUILD.bazel +++ b/submitqueue/orchestrator/controller/batch/BUILD.bazel @@ -15,7 +15,7 @@ go_library( "//submitqueue/core/request:go_default_library", "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -37,8 +37,9 @@ go_test( "//submitqueue/core/messagequeue:go_default_library", "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/batch/batch.go b/submitqueue/orchestrator/controller/batch/batch.go index 00d93a5d9..1613c2140 100644 --- a/submitqueue/orchestrator/controller/batch/batch.go +++ b/submitqueue/orchestrator/controller/batch/batch.go @@ -18,6 +18,8 @@ import ( "context" "fmt" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" "github.com/uber/submitqueue/platform/consumer" @@ -28,7 +30,6 @@ import ( corerequest "github.com/uber/submitqueue/submitqueue/core/request" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" "go.uber.org/zap" ) @@ -40,7 +41,7 @@ type Controller struct { metricsScope tally.Scope registry consumer.TopicRegistry counters counter.Factory - stores storage.Factory + stores orchstorage.Factory topicKey consumer.TopicKey consumerGroup string } @@ -61,7 +62,7 @@ func NewController( scope tally.Scope, registry consumer.TopicRegistry, counters counter.Factory, - stores storage.Factory, + stores orchstorage.Factory, topicKey consumer.TopicKey, consumerGroup string, ) *Controller { @@ -92,7 +93,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: rid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: rid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. diff --git a/submitqueue/orchestrator/controller/batch/batch_test.go b/submitqueue/orchestrator/controller/batch/batch_test.go index d597a42f3..f52b101f5 100644 --- a/submitqueue/orchestrator/controller/batch/batch_test.go +++ b/submitqueue/orchestrator/controller/batch/batch_test.go @@ -20,6 +20,8 @@ import ( "sync/atomic" "testing" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/uber-go/tally" @@ -34,8 +36,8 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -59,10 +61,10 @@ func newSequentialCounter(ctrl *gomock.Controller) *countermock.MockCounter { return cnt } -// storageFactoryFor returns a storage.Factory mock that resolves any queue to +// storageFactoryFor returns a orchstorage.Factory mock that resolves any queue to // the given queue-scoped store aggregate. -func storageFactoryFor(ctrl *gomock.Controller, store storage.Storage) *storagemock.MockFactory { - f := storagemock.NewMockFactory(ctrl) +func storageFactoryFor(ctrl *gomock.Controller, store orchstorage.Storage) *orchstoragemock.MockFactory { + f := orchstoragemock.NewMockFactory(ctrl) f.EXPECT().For(gomock.Any()).Return(store, nil).AnyTimes() return f } @@ -101,7 +103,7 @@ func newTestRegistry(t *testing.T, publisher *queuemock.MockPublisher, ctrl *gom // newTestController creates a controller with test dependencies. // If mockStorage is nil, a default MockStorage accepting any batch write is created. // handoffPublishErr, if non-nil, is returned for the hand-off publish. -func newTestController(t *testing.T, ctrl *gomock.Controller, cnt *countermock.MockCounter, mockStorage *storagemock.MockStorage, handoffPublishErr error) *Controller { +func newTestController(t *testing.T, ctrl *gomock.Controller, cnt *countermock.MockCounter, mockStorage *orchstoragemock.MockStorage, handoffPublishErr error) *Controller { logger := zaptest.NewLogger(t).Sugar() scope := tally.NoopScope @@ -113,7 +115,7 @@ func newTestController(t *testing.T, ctrl *gomock.Controller, cnt *countermock.M mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Get(gomock.Any(), req.ID).Return(req, nil).AnyTimes() - mockStorage = storagemock.NewMockStorage(ctrl) + mockStorage = orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() } @@ -165,7 +167,7 @@ func TestController_Process_Success(t *testing.T) { func TestController_Process_RejectsTenantPayloadQueueMismatch(t *testing.T) { ctrl := gomock.NewController(t) - controller := newTestController(t, ctrl, newSequentialCounter(ctrl), storagemock.NewMockStorage(ctrl), nil) + controller := newTestController(t, ctrl, newSequentialCounter(ctrl), orchstoragemock.NewMockStorage(ctrl), nil) request := testRequest() payload := requestIDPayload(t, request.ID, request.Queue) msg := entityqueue.NewMessage(request.ID, payload, request.Queue, nil) @@ -185,7 +187,7 @@ func TestController_Process_QueueMismatchRejected(t *testing.T) { mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Get(gomock.Any(), request.ID).Return(request, nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() mockStorage.EXPECT().GetBatchStore().Return(storagemock.NewMockBatchStore(ctrl)).AnyTimes() @@ -216,7 +218,7 @@ func TestController_Process_StampsQueueOnHandoffPayload(t *testing.T) { requestStore := storagemock.NewMockRequestStore(ctrl) requestStore.EXPECT().Get(gomock.Any(), request.ID).Return(request, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -241,7 +243,7 @@ func TestController_Process_StorageFailure(t *testing.T) { mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Get(gomock.Any(), request.ID).Return(entity.Request{}, fmt.Errorf("db connection lost")) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() mockStorage.EXPECT().GetBatchStore().Return(storagemock.NewMockBatchStore(ctrl)).AnyTimes() @@ -258,7 +260,7 @@ func TestController_Process_BatchStoreFailure(t *testing.T) { requestStore := storagemock.NewMockRequestStore(ctrl) requestStore.EXPECT().Get(gomock.Any(), request.ID).Return(request, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -303,7 +305,7 @@ func TestController_Process_HaltedShortCircuit(t *testing.T) { mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Get(gomock.Any(), request.ID).Return(request, nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(storagemock.NewMockBatchStore(ctrl)).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() @@ -344,7 +346,7 @@ func TestController_Process_WritesBatchBeforeHandoff(t *testing.T) { publisher.EXPECT().Publish(gomock.Any(), "dependency-analysis", gomock.Any()).Return(nil), ) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -377,7 +379,7 @@ func TestController_Process_CreatesBatchInCreatingWithoutDependencies(t *testing requestStore := storagemock.NewMockRequestStore(ctrl) requestStore.EXPECT().Get(gomock.Any(), request.ID).Return(request, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -403,7 +405,7 @@ func TestController_Process_PublishesBatchingStatus(t *testing.T) { requestStore := storagemock.NewMockRequestStore(ctrl) requestStore.EXPECT().Get(gomock.Any(), request.ID).Return(request, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() diff --git a/submitqueue/orchestrator/controller/build/BUILD.bazel b/submitqueue/orchestrator/controller/build/BUILD.bazel index cb4c9fad5..3879f6899 100644 --- a/submitqueue/orchestrator/controller/build/BUILD.bazel +++ b/submitqueue/orchestrator/controller/build/BUILD.bazel @@ -15,6 +15,7 @@ go_library( "//submitqueue/entity:go_default_library", "//submitqueue/extension/buildrunner:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -36,6 +37,8 @@ go_test( "//submitqueue/extension/buildrunner/mock:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/build/build.go b/submitqueue/orchestrator/controller/build/build.go index 5dc66aed4..59e724eb7 100644 --- a/submitqueue/orchestrator/controller/build/build.go +++ b/submitqueue/orchestrator/controller/build/build.go @@ -33,6 +33,8 @@ import ( "errors" "fmt" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" "github.com/uber/submitqueue/platform/consumer" @@ -51,7 +53,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory buildRunners buildrunner.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey @@ -68,7 +70,7 @@ const opName = "process" func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, buildRunners buildrunner.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, @@ -111,7 +113,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: bid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: bid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -217,7 +219,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // BuildRunner.Trigger accepts one, so a retry re-attaches to the existing build // instead of orphaning it. Only the runner can close this window, because the // build exists before anything here can write it down. -func (c *Controller) startPath(ctx context.Context, store storage.Storage, batch entity.Batch, entry entity.SpeculationPathEntry) error { +func (c *Controller) startPath(ctx context.Context, store orchstorage.Storage, batch entity.Batch, entry entity.SpeculationPathEntry) error { existing, err := store.GetPathBuildStore().Get(ctx, entry.ID, entry.Attempt) switch { case err == nil: @@ -324,7 +326,7 @@ func (c *Controller) startPath(ctx context.Context, store storage.Storage, batch // the queue's GC of consumed rows, which is why this runs only on redelivery: // scattered over every ordinary dispatch, late republishes would now and then // slip past dedup and fork a second, redundant poll chain for a healthy build. -func (c *Controller) ensureSignal(ctx context.Context, store storage.Storage, batch entity.Batch, entry entity.SpeculationPathEntry) error { +func (c *Controller) ensureSignal(ctx context.Context, store orchstorage.Storage, batch entity.Batch, entry entity.SpeculationPathEntry) error { link, err := store.GetPathBuildStore().Get(ctx, entry.ID, entry.Attempt) if errors.Is(err, storage.ErrNotFound) { // Nothing was dispatched for this attempt; there is no build to watch. @@ -350,7 +352,7 @@ func (c *Controller) ensureSignal(ctx context.Context, store storage.Storage, ba // Which dependencies those are is the path's own to say — see // SpeculationPath.Base — so this only resolves the IDs it is // given. Nothing about assumptions is interpreted here. -func (c *Controller) loadBase(ctx context.Context, store storage.Storage, path entity.SpeculationPath) ([]entity.Batch, error) { +func (c *Controller) loadBase(ctx context.Context, store orchstorage.Storage, path entity.SpeculationPath) ([]entity.Batch, error) { deps := path.Base() if len(deps) == 0 { return nil, nil diff --git a/submitqueue/orchestrator/controller/build/build_test.go b/submitqueue/orchestrator/controller/build/build_test.go index 0dd3f1305..96c9c51e3 100644 --- a/submitqueue/orchestrator/controller/build/build_test.go +++ b/submitqueue/orchestrator/controller/build/build_test.go @@ -19,6 +19,8 @@ import ( "fmt" "testing" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/uber-go/tally" @@ -33,15 +35,18 @@ import ( buildrunnermock "github.com/uber/submitqueue/submitqueue/extension/buildrunner/mock" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} const ( headID = "test-queue/batch/head" @@ -97,7 +102,7 @@ func pathEntry(status entity.SpeculationPathStatus, attempt int) entity.Speculat // testDeps holds the mocks a test may want to set expectations on. type testDeps struct { - store *storagemock.MockStorage + store *orchstoragemock.MockStorage batches *storagemock.MockBatchStore pathSets *storagemock.MockSpeculationPathSetStore builds *storagemock.MockBuildStore @@ -122,7 +127,7 @@ func newTestController(t *testing.T, ctrl *gomock.Controller, batch entity.Batch builds := storagemock.NewMockBuildStore(ctrl) pathBuilds := storagemock.NewMockPathBuildStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batches).AnyTimes() store.EXPECT().GetSpeculationPathSetStore().Return(pathSets).AnyTimes() store.EXPECT().GetBuildStore().Return(builds).AnyTimes() diff --git a/submitqueue/orchestrator/controller/buildsignal/BUILD.bazel b/submitqueue/orchestrator/controller/buildsignal/BUILD.bazel index fb40c8c2f..50fda4307 100644 --- a/submitqueue/orchestrator/controller/buildsignal/BUILD.bazel +++ b/submitqueue/orchestrator/controller/buildsignal/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//submitqueue/entity:go_default_library", "//submitqueue/extension/buildrunner:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -36,6 +37,8 @@ go_test( "//submitqueue/extension/buildrunner/mock:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/buildsignal/buildsignal.go b/submitqueue/orchestrator/controller/buildsignal/buildsignal.go index 65bf4c990..89fee0ae6 100644 --- a/submitqueue/orchestrator/controller/buildsignal/buildsignal.go +++ b/submitqueue/orchestrator/controller/buildsignal/buildsignal.go @@ -55,7 +55,8 @@ import ( "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/buildrunner" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -82,7 +83,7 @@ const opName = "process" type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory buildRunners buildrunner.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey @@ -96,7 +97,7 @@ var _ consumer.Controller = (*Controller)(nil) func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, buildRunners buildrunner.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, @@ -149,7 +150,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: buildID.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: buildID.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -361,7 +362,7 @@ func (c *Controller) publishBuildLogs( // therefore indicate store corruption, and since a cancel is irreversible, a // corrupt kill list keeps the build rather than killing it; a halted batch is // still caught by the first check, which needs none of those records. -func (c *Controller) unwanted(ctx context.Context, store storage.Storage, batch entity.Batch, build entity.Build) (bool, error) { +func (c *Controller) unwanted(ctx context.Context, store orchstorage.Storage, batch entity.Batch, build entity.Build) (bool, error) { if entity.IsBatchStateHalted(batch.State) { return true, nil } diff --git a/submitqueue/orchestrator/controller/buildsignal/buildsignal_test.go b/submitqueue/orchestrator/controller/buildsignal/buildsignal_test.go index 543531a67..c2c812193 100644 --- a/submitqueue/orchestrator/controller/buildsignal/buildsignal_test.go +++ b/submitqueue/orchestrator/controller/buildsignal/buildsignal_test.go @@ -31,8 +31,10 @@ import ( "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" buildrunnermock "github.com/uber/submitqueue/submitqueue/extension/buildrunner/mock" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -64,10 +66,12 @@ type testHarness struct { } // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} // testRequestID is the one member of the batch under test, so the request-log // fan-out has somebody to report to. @@ -120,7 +124,7 @@ func newTestHarness(t *testing.T, ctrl *gomock.Controller, batchState entity.Bat pathSets := storagemock.NewMockSpeculationPathSetStore(ctrl) pathBuilds := storagemock.NewMockPathBuildStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBuildStore().Return(builds).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetSpeculationPathSetStore().Return(pathSets).AnyTimes() diff --git a/submitqueue/orchestrator/controller/cancel/BUILD.bazel b/submitqueue/orchestrator/controller/cancel/BUILD.bazel index 3ab90a062..98701cf59 100644 --- a/submitqueue/orchestrator/controller/cancel/BUILD.bazel +++ b/submitqueue/orchestrator/controller/cancel/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -35,6 +36,8 @@ go_test( "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/cancel/cancel.go b/submitqueue/orchestrator/controller/cancel/cancel.go index 786ea106c..1c9897658 100644 --- a/submitqueue/orchestrator/controller/cancel/cancel.go +++ b/submitqueue/orchestrator/controller/cancel/cancel.go @@ -64,7 +64,8 @@ import ( corerequest "github.com/uber/submitqueue/submitqueue/core/request" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -72,7 +73,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -87,7 +88,7 @@ const opName = "process" func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -116,7 +117,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: cancelReq.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: cancelReq.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -211,7 +212,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // observing a batch transition) is returned as-is; its declaration makes it // retryable, and the next attempt re-fetches and re-evaluates (it may now // be terminal, in which case the top-level terminal-check acks). -func (c *Controller) markCancelling(ctx context.Context, store storage.Storage, request entity.Request) (entity.Request, error) { +func (c *Controller) markCancelling(ctx context.Context, store orchstorage.Storage, request entity.Request) (entity.Request, error) { if request.State == entity.RequestStateCancelling { // Idempotent re-delivery: prior pass already recorded intent. metrics.NamedCounter(c.metricsScope, opName, "already_cancelling", 1) @@ -230,7 +231,7 @@ func (c *Controller) markCancelling(ctx context.Context, store storage.Storage, // findBatches resolves every batch attempt associated with the request. // Associations whose batch was never persisted are stale retry artifacts and are ignored. -func (c *Controller) findBatches(ctx context.Context, store storage.Storage, request entity.Request) ([]entity.Batch, error) { +func (c *Controller) findBatches(ctx context.Context, store orchstorage.Storage, request entity.Request) ([]entity.Batch, error) { batches, stale, err := corebatch.FindByRequestID(ctx, store, request.ID) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "batch_lookup_errors", 1) @@ -255,7 +256,7 @@ func (c *Controller) findBatches(ctx context.Context, store storage.Storage, req // concurrent writer already reached a *different* terminal state, the helper // reports TerminationDiverged and we simply ack — the other writer owns the // terminal log for the state it wrote. -func (c *Controller) cancelRequest(ctx context.Context, store storage.Storage, request entity.Request, reason string) error { +func (c *Controller) cancelRequest(ctx context.Context, store orchstorage.Storage, request entity.Request, reason string) error { metadata := map[string]string{} if reason != "" { metadata["reason"] = reason @@ -303,7 +304,7 @@ func (c *Controller) cancelRequest(ctx context.Context, store storage.Storage, r // (a prior pass wrote the intent but the publish failed). In that case the // intent CAS is skipped and we just re-publish — speculate absorbs the // duplicate as a cheap no-op nudge. -func (c *Controller) cancelBatch(ctx context.Context, store storage.Storage, batch entity.Batch) error { +func (c *Controller) cancelBatch(ctx context.Context, store orchstorage.Storage, batch entity.Batch) error { c.logger.Infow("handing batch cancellation off to speculate", "batch_id", batch.ID, "queue", batch.Queue, diff --git a/submitqueue/orchestrator/controller/cancel/cancel_test.go b/submitqueue/orchestrator/controller/cancel/cancel_test.go index 32a400c59..a7c3f7feb 100644 --- a/submitqueue/orchestrator/controller/cancel/cancel_test.go +++ b/submitqueue/orchestrator/controller/cancel/cancel_test.go @@ -29,8 +29,10 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -38,10 +40,12 @@ import ( // newQueueBatchStateStore returns a QueueBatchStateStore mock that accepts any // membership-record write; cancel never lists record buckets. // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} func newQueueBatchStateStore(ctrl *gomock.Controller) *storagemock.MockQueueBatchStateStore { s := storagemock.NewMockQueueBatchStateStore(ctrl) @@ -86,7 +90,7 @@ func newRegistry(t *testing.T, ctrl *gomock.Controller) (consumer.TopicRegistry, return reg, pub } -func newController(t *testing.T, store storage.Storage, registry consumer.TopicRegistry) *Controller { +func newController(t *testing.T, store orchstorage.Storage, registry consumer.TopicRegistry) *Controller { return NewController(zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: store}, registry, topickey.TopicKeyCancel, "orchestrator-cancel") } @@ -101,7 +105,7 @@ func newDelivery(t *testing.T, ctrl *gomock.Controller, payload []byte, partitio func expectBatchLookup( ctrl *gomock.Controller, - store *storagemock.MockStorage, + store *orchstoragemock.MockStorage, batchStore *storagemock.MockBatchStore, requestID string, batches ...entity.Batch, @@ -126,7 +130,7 @@ func TestNewController(t *testing.T) { registry, pub := newRegistry(t, ctrl) pub.EXPECT().Publish(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() controller := newController(t, store, registry) @@ -140,7 +144,7 @@ func TestNewController(t *testing.T) { func TestProcess_RejectsTenantPayloadQueueMismatch(t *testing.T) { ctrl := gomock.NewController(t) - controller := newController(t, storagemock.NewMockStorage(ctrl), consumer.TopicRegistry{}) + controller := newController(t, orchstoragemock.NewMockStorage(ctrl), consumer.TopicRegistry{}) msg := entityqueue.NewMessage("cancel-msg", cancelPayload(t, "q/1", ""), "q", nil) msg.Tenant = "other-queue" d := consumermock.NewMockDelivery(ctrl) @@ -160,7 +164,7 @@ func TestProcess_AlreadyTerminal_NoOp(t *testing.T) { ID: "q/1", Queue: "q", State: entity.RequestStateCancelled, Version: 5, }, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() @@ -176,7 +180,7 @@ func TestProcess_RequestNotFound_Retryable(t *testing.T) { reqStore := storagemock.NewMockRequestStore(ctrl) reqStore.EXPECT().Get(gomock.Any(), "q/1").Return(entity.Request{}, storage.ErrNotFound) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() @@ -211,7 +215,7 @@ func TestProcess_CancelsUnbatchedRequest(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -244,7 +248,7 @@ func TestProcess_AlreadyCancelling_SkipsMarkCancelling(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -273,7 +277,7 @@ func TestProcess_MarkCancellingVersionMismatch_Retryable(t *testing.T) { reqStore.EXPECT().Update(gomock.Any(), requestWithState(started, entity.RequestStateCancelling), int32(2), int32(3)). Return(storage.ErrVersionMismatch) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() @@ -303,7 +307,7 @@ func TestProcess_UnbatchedVersionMismatch_Retryable(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -331,7 +335,7 @@ func TestProcess_UnbatchedRequestDiverged_Acks(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -357,7 +361,7 @@ func TestProcess_UnbatchedRequestDisappears_Retryable(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -416,7 +420,7 @@ func TestProcess_BatchPath_HandsOffToSpeculate(t *testing.T) { // Single batch CAS: intent only. No terminal CAS. batchStore.EXPECT().Update(gomock.Any(), batchWithState(batch, entity.BatchStateCancelling), int32(3), int32(4)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -473,7 +477,7 @@ func TestProcess_CancelsEveryApplicableBatch(t *testing.T) { }, ).Times(2) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -514,7 +518,7 @@ func TestProcess_BatchFailureDoesNotPreventLaterCancellation(t *testing.T) { }, ) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -550,7 +554,7 @@ func TestProcess_NonCancellableBatchSuppressesRequestCancellation(t *testing.T) batchStore := storagemock.NewMockBatchStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -580,7 +584,7 @@ func TestProcess_BatchedWithoutMatchCancelsRequest(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -615,7 +619,7 @@ func TestProcess_CreatingBatchDoesNotSuppressRequestCancellation(t *testing.T) { ) batchStore := storagemock.NewMockBatchStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -660,7 +664,7 @@ func TestProcess_BatchAlreadyCancelling_RepublishesToSpeculate(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) // No batch Update — already in Cancelling. - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -692,7 +696,7 @@ func TestProcess_BatchIntentVersionMismatch_Retryable(t *testing.T) { batchStore.EXPECT().Update(gomock.Any(), batchWithState(batch, entity.BatchStateCancelling), int32(1), int32(2)). Return(storage.ErrVersionMismatch) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -708,7 +712,7 @@ func TestProcess_DeserializeError(t *testing.T) { ctrl := gomock.NewController(t) registry, _ := newRegistry(t, ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() controller := newController(t, store, registry) err := controller.Process(context.Background(), newDelivery(t, ctrl, []byte("not json"), "q/1")) @@ -722,7 +726,7 @@ func TestProcess_RequestStoreError(t *testing.T) { reqStore := storagemock.NewMockRequestStore(ctrl) reqStore.EXPECT().Get(gomock.Any(), "q/1").Return(entity.Request{}, fmt.Errorf("db down")) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() @@ -794,7 +798,7 @@ func TestFindBatches(t *testing.T) { requestBatchStore := storagemock.NewMockRequestBatchStore(ctrl) tt.mockFunc(requestBatchStore, batchStore) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(requestBatchStore) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() diff --git a/submitqueue/orchestrator/controller/conclude/BUILD.bazel b/submitqueue/orchestrator/controller/conclude/BUILD.bazel index 44a064048..29f024889 100644 --- a/submitqueue/orchestrator/controller/conclude/BUILD.bazel +++ b/submitqueue/orchestrator/controller/conclude/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -34,6 +35,8 @@ go_test( "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/conclude/conclude.go b/submitqueue/orchestrator/controller/conclude/conclude.go index 5b99d0660..07bb56164 100644 --- a/submitqueue/orchestrator/controller/conclude/conclude.go +++ b/submitqueue/orchestrator/controller/conclude/conclude.go @@ -26,7 +26,8 @@ import ( corerequest "github.com/uber/submitqueue/submitqueue/core/request" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -36,7 +37,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -49,7 +50,7 @@ var _ consumer.Controller = (*Controller)(nil) func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -79,7 +80,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: bid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: bid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, "process", "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. diff --git a/submitqueue/orchestrator/controller/conclude/conclude_test.go b/submitqueue/orchestrator/controller/conclude/conclude_test.go index 604187cdf..3296e2dcb 100644 --- a/submitqueue/orchestrator/controller/conclude/conclude_test.go +++ b/submitqueue/orchestrator/controller/conclude/conclude_test.go @@ -30,17 +30,21 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} func requestWithState(request entity.Request, state entity.RequestState) entity.Request { request.State = state @@ -58,14 +62,14 @@ func batchIDPayload(t *testing.T, id string) []byte { // expectLogPublish controls whether the log topic publisher is wired with an // expectation; tests that don't reach the log publish step pass false so an // unexpected publish would fail the test. -func newTestController(t *testing.T, ctrl *gomock.Controller, mockStorage *storagemock.MockStorage, expectLogPublish bool) (*Controller, *queuemock.MockPublisher) { +func newTestController(t *testing.T, ctrl *gomock.Controller, mockStorage *orchstoragemock.MockStorage, expectLogPublish bool) (*Controller, *queuemock.MockPublisher) { logger := zaptest.NewLogger(t).Sugar() scope := tally.NoopScope if mockStorage == nil { mockRequestStore := storagemock.NewMockRequestStore(ctrl) mockBatchStore := storagemock.NewMockBatchStore(ctrl) - mockStorage = storagemock.NewMockStorage(ctrl) + mockStorage = orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() } @@ -112,7 +116,7 @@ func TestController_Process(t *testing.T) { tests := []struct { name string batch entity.Batch - setupStore func(*gomock.Controller) *storagemock.MockStorage + setupStore func(*gomock.Controller) *orchstoragemock.MockStorage expectLogPublish bool wantErr bool retryable bool @@ -126,7 +130,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateSucceeded, Version: 3, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/1").Return(entity.Batch{ ID: "test-queue/batch/1", @@ -148,7 +152,7 @@ func TestController_Process(t *testing.T) { mockRequestStore.EXPECT().Get(gomock.Any(), "test-queue/2").Return(request2, nil) mockRequestStore.EXPECT().Update(gomock.Any(), requestWithState(request2, entity.RequestStateLanded), int32(3), int32(4)).Return(nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() return mockStorage @@ -164,7 +168,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateFailed, Version: 2, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/2").Return(entity.Batch{ ID: "test-queue/batch/2", @@ -181,7 +185,7 @@ func TestController_Process(t *testing.T) { mockRequestStore.EXPECT().Get(gomock.Any(), "test-queue/5").Return(request, nil) mockRequestStore.EXPECT().Update(gomock.Any(), requestWithState(request, entity.RequestStateError), int32(1), int32(2)).Return(nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() return mockStorage @@ -197,7 +201,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateCancelled, Version: 2, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/3").Return(entity.Batch{ ID: "test-queue/batch/3", @@ -214,7 +218,7 @@ func TestController_Process(t *testing.T) { mockRequestStore.EXPECT().Get(gomock.Any(), "test-queue/10").Return(request, nil) mockRequestStore.EXPECT().Update(gomock.Any(), requestWithState(request, entity.RequestStateCancelled), int32(4), int32(5)).Return(nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() return mockStorage @@ -230,7 +234,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateSucceeded, Version: 2, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/8").Return(entity.Batch{ ID: "test-queue/batch/8", @@ -247,7 +251,7 @@ func TestController_Process(t *testing.T) { ID: "test-queue/20", Queue: "test-queue", Version: 7, State: entity.RequestStateLanded, }, nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() return mockStorage @@ -263,7 +267,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateSucceeded, Version: 2, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/9").Return(entity.Batch{ ID: "test-queue/batch/9", @@ -281,7 +285,7 @@ func TestController_Process(t *testing.T) { ID: "test-queue/30", Queue: "test-queue", Version: 5, State: entity.RequestStateCancelled, }, nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() return mockStorage @@ -297,7 +301,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateSucceeded, Version: 2, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/11").Return(entity.Batch{ ID: "test-queue/batch/11", @@ -312,7 +316,7 @@ func TestController_Process(t *testing.T) { mockRequestStore := storagemock.NewMockRequestStore(ctrl) mockRequestStore.EXPECT().Get(gomock.Any(), "test-queue/40").Return(entity.Request{}, storage.ErrNotFound) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() return mockStorage @@ -329,7 +333,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateCreated, Version: 1, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/4").Return(entity.Batch{ ID: "test-queue/batch/4", @@ -339,7 +343,7 @@ func TestController_Process(t *testing.T) { Version: 1, }, nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() return mockStorage }, @@ -355,7 +359,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateSucceeded, Version: 2, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/5").Return(entity.Batch{ ID: "test-queue/batch/5", @@ -368,7 +372,7 @@ func TestController_Process(t *testing.T) { mockRequestStore := storagemock.NewMockRequestStore(ctrl) mockRequestStore.EXPECT().Get(gomock.Any(), "test-queue/1").Return(entity.Request{}, fmt.Errorf("db connection lost")) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() return mockStorage @@ -385,7 +389,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateSucceeded, Version: 2, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/6").Return(entity.Batch{ ID: "test-queue/batch/6", @@ -402,7 +406,7 @@ func TestController_Process(t *testing.T) { mockRequestStore.EXPECT().Get(gomock.Any(), "test-queue/1").Return(request, nil) mockRequestStore.EXPECT().Update(gomock.Any(), requestWithState(request, entity.RequestStateLanded), int32(2), int32(3)).Return(storage.ErrVersionMismatch) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() mockStorage.EXPECT().GetRequestStore().Return(mockRequestStore).AnyTimes() return mockStorage @@ -418,7 +422,7 @@ func TestController_Process(t *testing.T) { State: entity.BatchStateSucceeded, Version: 1, }, - setupStore: func(ctrl *gomock.Controller) *storagemock.MockStorage { + setupStore: func(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/7").Return(entity.Batch{ ID: "test-queue/batch/7", @@ -427,7 +431,7 @@ func TestController_Process(t *testing.T) { Version: 1, }, nil) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() return mockStorage }, @@ -438,7 +442,7 @@ func TestController_Process(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctrl := gomock.NewController(t) - var mockStorage *storagemock.MockStorage + var mockStorage *orchstoragemock.MockStorage if tt.setupStore != nil { mockStorage = tt.setupStore(ctrl) } @@ -487,7 +491,7 @@ func TestController_Process_FailedBatchCarriesReasonToRequestLog(t *testing.T) { requestStore.EXPECT().Get(gomock.Any(), "test-queue/9").Return(request, nil) requestStore.EXPECT().Update(gomock.Any(), requestWithState(request, entity.RequestStateError), int32(1), int32(2)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -521,7 +525,7 @@ func TestController_Process_StorageFailure(t *testing.T) { mockBatchStore := storagemock.NewMockBatchStore(ctrl) mockBatchStore.EXPECT().Get(gomock.Any(), "test-queue/batch/1").Return(entity.Batch{}, fmt.Errorf("db connection lost")) - mockStorage := storagemock.NewMockStorage(ctrl) + mockStorage := orchstoragemock.NewMockStorage(ctrl) mockStorage.EXPECT().GetBatchStore().Return(mockBatchStore).AnyTimes() controller, _ := newTestController(t, ctrl, mockStorage, false) diff --git a/submitqueue/orchestrator/controller/dependencyanalysis/BUILD.bazel b/submitqueue/orchestrator/controller/dependencyanalysis/BUILD.bazel index a8d24de8c..be0b1a537 100644 --- a/submitqueue/orchestrator/controller/dependencyanalysis/BUILD.bazel +++ b/submitqueue/orchestrator/controller/dependencyanalysis/BUILD.bazel @@ -17,6 +17,7 @@ go_library( "//submitqueue/entity:go_default_library", "//submitqueue/extension/conflict:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -39,6 +40,8 @@ go_test( "//submitqueue/extension/conflict/mock:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go b/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go index b018972aa..c168239d5 100644 --- a/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go +++ b/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go @@ -64,7 +64,8 @@ import ( "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/conflict" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -73,7 +74,7 @@ type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope registry consumer.TopicRegistry - stores storage.Factory + stores orchstorage.Factory analyzers conflict.Factory topicKey consumer.TopicKey consumerGroup string @@ -88,7 +89,7 @@ const opName = "process" func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, analyzers conflict.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, @@ -120,7 +121,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: bid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: bid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -252,7 +253,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // partitioned by queue and consumed in order, so the first hand-off through // here enrols the request and the second finds it and stops. Without the check // the same change would end up in two live batches, both admitted, both landed. -func (c *Controller) requestEnrolledInAnotherBatch(ctx context.Context, store storage.Storage, batch entity.Batch) (bool, error) { +func (c *Controller) requestEnrolledInAnotherBatch(ctx context.Context, store orchstorage.Storage, batch entity.Batch) (bool, error) { for _, requestID := range batch.Contains { existing, stale, err := corebatch.FindByRequestID(ctx, store, requestID) if err != nil { @@ -280,7 +281,7 @@ func (c *Controller) requestEnrolledInAnotherBatch(ctx context.Context, store st // associateRequestsWithBatch links the batch to its requests. This is the record that makes the // batch findable from a request, so writing it is what enrols them. -func (c *Controller) associateRequestsWithBatch(ctx context.Context, store storage.Storage, batch entity.Batch) error { +func (c *Controller) associateRequestsWithBatch(ctx context.Context, store orchstorage.Storage, batch entity.Batch) error { for _, requestID := range batch.Contains { association := entity.RequestBatch{RequestID: requestID, BatchID: batch.ID, Version: 1} if err := store.GetRequestBatchStore().Create(ctx, association); err != nil && !errors.Is(err, storage.ErrAlreadyExists) { @@ -305,7 +306,7 @@ func (c *Controller) associateRequestsWithBatch(ctx context.Context, store stora // // A redelivery re-applies Batched → Batched as a version-only bump, which keeps // both guards in force on every attempt. -func (c *Controller) claimRequestsForBatch(ctx context.Context, store storage.Storage, batch entity.Batch) (bool, error) { +func (c *Controller) claimRequestsForBatch(ctx context.Context, store orchstorage.Storage, batch entity.Batch) (bool, error) { for _, requestID := range batch.Contains { request, err := store.GetRequestStore().Get(ctx, requestID) if err != nil { @@ -344,7 +345,7 @@ func (c *Controller) claimRequestsForBatch(ctx context.Context, store storage.St // firstHaltedRequestID returns the first request in the batch the user has given up // on, or the empty string if every member is still live. -func (c *Controller) firstHaltedRequestID(ctx context.Context, store storage.Storage, batch entity.Batch) (string, error) { +func (c *Controller) firstHaltedRequestID(ctx context.Context, store orchstorage.Storage, batch entity.Batch) (string, error) { for _, requestID := range batch.Contains { request, err := store.GetRequestStore().Get(ctx, requestID) if err != nil { @@ -362,7 +363,7 @@ func (c *Controller) firstHaltedRequestID(ctx context.Context, store storage.Sto // batch must serialize behind. The read goes through the queue's per-state // membership records; classification uses each batch's own hydrated state, so // a stale record can never misreport a batch. -func (c *Controller) resolveDependencies(ctx context.Context, store storage.Storage, batch entity.Batch) ([]string, error) { +func (c *Controller) resolveDependencies(ctx context.Context, store orchstorage.Storage, batch entity.Batch) ([]string, error) { inFlight, err := corebatch.ListByStates(ctx, store, entity.DependencyBatchStates()) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "batch_store_errors", 1) @@ -401,7 +402,7 @@ func (c *Controller) resolveDependencies(ctx context.Context, store storage.Stor // Both writes are idempotent on their own: a failure part-way through the loop // leaves the batch in Creating, so the retry re-enters here and would // otherwise duplicate whatever the first pass already wrote. -func (c *Controller) writeDependentIndexes(ctx context.Context, store storage.Storage, batch entity.Batch, dependencies []string) error { +func (c *Controller) writeDependentIndexes(ctx context.Context, store orchstorage.Storage, batch entity.Batch, dependencies []string) error { own := entity.BatchDependent{ BatchID: batch.ID, Dependents: []string{}, diff --git a/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis_test.go b/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis_test.go index 514635db0..9748afb53 100644 --- a/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis_test.go +++ b/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis_test.go @@ -33,8 +33,10 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/conflict" "github.com/uber/submitqueue/submitqueue/extension/conflict/all" conflictmock "github.com/uber/submitqueue/submitqueue/extension/conflict/mock" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -100,8 +102,8 @@ func newQueueBatchStateStore(ctrl *gomock.Controller, active ...entity.Batch) *s return s } -func storageFactoryFor(ctrl *gomock.Controller, store storage.Storage) *storagemock.MockFactory { - f := storagemock.NewMockFactory(ctrl) +func storageFactoryFor(ctrl *gomock.Controller, store orchstorage.Storage) *orchstoragemock.MockFactory { + f := orchstoragemock.NewMockFactory(ctrl) f.EXPECT().For(gomock.Any()).Return(store, nil).AnyTimes() return f } @@ -109,7 +111,7 @@ func storageFactoryFor(ctrl *gomock.Controller, store storage.Storage) *storagem // newTestController builds a controller over the given store. A nil analyzer // resolves to the "all" analyzer, under which every dependency-eligible batch // conflicts. -func newTestController(t *testing.T, ctrl *gomock.Controller, store storage.Storage, analyzer conflict.Analyzer, publisher *queuemock.MockPublisher) *Controller { +func newTestController(t *testing.T, ctrl *gomock.Controller, store orchstorage.Storage, analyzer conflict.Analyzer, publisher *queuemock.MockPublisher) *Controller { t.Helper() if analyzer == nil { @@ -167,7 +169,7 @@ func noPriorEnrollment(ctrl *gomock.Controller) *storagemock.MockRequestBatchSto func TestNewController(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) controller := newTestController(t, ctrl, store, nil, nil) require.NotNil(t, controller) @@ -180,7 +182,7 @@ func TestNewController(t *testing.T) { func TestController_Process_RejectsTenantPayloadQueueMismatch(t *testing.T) { ctrl := gomock.NewController(t) - controller := newTestController(t, ctrl, storagemock.NewMockStorage(ctrl), nil, nil) + controller := newTestController(t, ctrl, orchstoragemock.NewMockStorage(ctrl), nil, nil) msg := entityqueue.NewMessage("test-queue/batch/1", batchIDPayload(t, "test-queue/batch/1", "test-queue"), "test-queue", nil) msg.Tenant = "other-queue" delivery := consumermock.NewMockDelivery(ctrl) @@ -239,7 +241,7 @@ func TestController_Process_AnalyzesAndTransitionsToCreated(t *testing.T) { Version: 2, }, int32(2), int32(3)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl, inFlight...)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -282,7 +284,7 @@ func TestController_Process_DedupesAnalyzerConflicts(t *testing.T) { Version: 5, }, int32(5), int32(6)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl, inFlight)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -313,7 +315,7 @@ func TestController_Process_RedeliveryAfterTransitionOnlyRepublishes(t *testing. batchStore.EXPECT().Get(gomock.Any(), batch.ID).Return(batch, nil) // Dependent store and request store with no EXPECTs — must not be touched. - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(storagemock.NewMockBatchDependentStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(storagemock.NewMockRequestStore(ctrl)).AnyTimes() @@ -349,7 +351,7 @@ func TestController_Process_RedeliveryMidIndexDoesNotDoubleAppend(t *testing.T) Version: 2, }, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl, inFlight)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -378,7 +380,7 @@ func TestController_Process_HaltedRequestIsNotPromoted(t *testing.T) { batchStore.EXPECT().Get(gomock.Any(), batch.ID).Return(batch, nil) // Dependent store with no EXPECTs — must not be touched. - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(storagemock.NewMockBatchDependentStore(ctrl)).AnyTimes() @@ -408,7 +410,7 @@ func TestController_Process_HaltedBatchAcksWithoutPublishing(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batchStore.EXPECT().Get(gomock.Any(), batch.ID).Return(batch, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(storagemock.NewMockRequestStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noPriorEnrollment(ctrl)).AnyTimes() @@ -433,7 +435,7 @@ func TestController_Process_AlreadyAdmittedAcksWithoutPublishing(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batchStore.EXPECT().Get(gomock.Any(), batch.ID).Return(batch, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(storagemock.NewMockRequestStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noPriorEnrollment(ctrl)).AnyTimes() @@ -451,7 +453,7 @@ func TestController_Process_QueueMismatchRejected(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batchStore.EXPECT().Get(gomock.Any(), batch.ID).Return(batch, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(storagemock.NewMockRequestStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noPriorEnrollment(ctrl)).AnyTimes() @@ -474,7 +476,7 @@ func TestController_Process_StampsQueueOnAnnouncement(t *testing.T) { dependentStore := storagemock.NewMockBatchDependentStore(ctrl) dependentStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -510,7 +512,7 @@ func TestController_Process_AnalyzerFailure(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batchStore.EXPECT().Get(gomock.Any(), batch.ID).Return(batch, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStoreFor(ctrl, liveRequest())).AnyTimes() @@ -548,7 +550,7 @@ func TestController_Process_IndexUpdateFailureDoesNotMutateFetchedDependents(t * Version: existing.Version, }, existing.Version, existing.Version+1).Return(errors.New("update failed")) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl, inFlight)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -619,7 +621,7 @@ func TestController_Process_PromotionErrors(t *testing.T) { dependentStore := storagemock.NewMockBatchDependentStore(ctrl) tt.setup(batchStore, dependentStore) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl, inFlight)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -647,7 +649,7 @@ func TestController_Process_PublishesBatchedLogOnPromotion(t *testing.T) { dependentStore := storagemock.NewMockBatchDependentStore(ctrl) dependentStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -698,7 +700,7 @@ func TestController_Process_AbandonsBatchWhoseRequestIsAlreadyEnrolled(t *testin }, nil) // Dependent store and request store with no EXPECTs — neither may be touched. - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(associations).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(storagemock.NewMockBatchDependentStore(ctrl)).AnyTimes() @@ -728,7 +730,7 @@ func TestController_Process_OwnAssociationDoesNotBlockPromotion(t *testing.T) { }, nil) associations.EXPECT().Create(gomock.Any(), gomock.Any()).Return(storage.ErrAlreadyExists) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -766,7 +768,7 @@ func TestController_Process_EnrolsTheRequest(t *testing.T) { requestStore.EXPECT().Get(gomock.Any(), testRequestID).Return(request, nil).AnyTimes() requestStore.EXPECT().Update(gomock.Any(), claimed, request.Version, request.Version+1).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -800,7 +802,7 @@ func TestController_Process_ClaimLostToCancelAbandonsBatch(t *testing.T) { requestStore.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(fmt.Errorf("cas: %w", storage.ErrVersionMismatch)) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -841,7 +843,7 @@ func TestController_Process_RequestCancelledDuringAnalysisAbandonsBatch(t *testi ) // Update must NOT be called — claiming would overwrite the cancellation. - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() @@ -874,7 +876,7 @@ func TestController_Process_ClaimStorageErrorPropagates(t *testing.T) { requestStore.EXPECT().Get(gomock.Any(), testRequestID).Return(liveRequest(), nil).AnyTimes() requestStore.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(claimErr) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetBatchDependentStore().Return(dependentStore).AnyTimes() diff --git a/submitqueue/orchestrator/controller/dlq/BUILD.bazel b/submitqueue/orchestrator/controller/dlq/BUILD.bazel index 7f839936e..4859a31b7 100644 --- a/submitqueue/orchestrator/controller/dlq/BUILD.bazel +++ b/submitqueue/orchestrator/controller/dlq/BUILD.bazel @@ -26,6 +26,7 @@ go_library( "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -59,6 +60,8 @@ go_test( "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/dlq/batch.go b/submitqueue/orchestrator/controller/dlq/batch.go index c790e84d5..f2f9fca72 100644 --- a/submitqueue/orchestrator/controller/dlq/batch.go +++ b/submitqueue/orchestrator/controller/dlq/batch.go @@ -18,12 +18,13 @@ import ( "context" "fmt" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" "github.com/uber/submitqueue/platform/consumer" "github.com/uber/submitqueue/platform/metrics" sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" - "github.com/uber/submitqueue/submitqueue/extension/storage" "go.uber.org/zap" ) @@ -46,7 +47,7 @@ import ( type batchController struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -60,7 +61,7 @@ var _ consumer.Controller = (*batchController)(nil) func NewDLQBatchController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -96,7 +97,7 @@ func (c *batchController) Process(ctx context.Context, delivery consumer.Deliver return fmt.Errorf("dlq payload decoded to empty batch id") } - store, err := c.stores.For(storage.Config{QueueName: bid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: bid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. diff --git a/submitqueue/orchestrator/controller/dlq/batch_test.go b/submitqueue/orchestrator/controller/dlq/batch_test.go index 544e67588..048a0437d 100644 --- a/submitqueue/orchestrator/controller/dlq/batch_test.go +++ b/submitqueue/orchestrator/controller/dlq/batch_test.go @@ -25,13 +25,14 @@ import ( "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) func TestDLQBatchController_InterfaceAndAccessors(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyLand), "orchestrator-land-dlq") @@ -63,7 +64,7 @@ func TestDLQBatchController_Process_FailsAndFansOut(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -79,7 +80,7 @@ func TestDLQBatchController_Process_FailsAndFansOut(t *testing.T) { func TestDLQBatchController_Process_TenantPayloadQueueMismatchAcks(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyLand), "orchestrator-land-dlq") payload, err := sqmq.MarshalID(sqmq.TopicKeyLand, "q/batch/9", "q") @@ -91,7 +92,7 @@ func TestDLQBatchController_Process_TenantPayloadQueueMismatchAcks(t *testing.T) func TestDLQBatchController_Process_MalformedPayloadFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyLand), "orchestrator-land-dlq") @@ -103,7 +104,7 @@ func TestDLQBatchController_Process_MalformedPayloadFails(t *testing.T) { func TestDLQBatchController_Process_EmptyIDFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyLand), "orchestrator-land-dlq") diff --git a/submitqueue/orchestrator/controller/dlq/buildsignal.go b/submitqueue/orchestrator/controller/dlq/buildsignal.go index a25ab4aa9..500cb6c4b 100644 --- a/submitqueue/orchestrator/controller/dlq/buildsignal.go +++ b/submitqueue/orchestrator/controller/dlq/buildsignal.go @@ -19,6 +19,8 @@ import ( "errors" "fmt" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" "github.com/uber/submitqueue/platform/consumer" @@ -39,7 +41,7 @@ import ( type buildSignalController struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -52,7 +54,7 @@ var _ consumer.Controller = (*buildSignalController)(nil) func NewDLQBuildSignalController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -88,7 +90,7 @@ func (c *buildSignalController) Process(ctx context.Context, delivery consumer.D return fmt.Errorf("dlq payload decoded to empty build id") } - store, err := c.stores.For(storage.Config{QueueName: buildID.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: buildID.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. diff --git a/submitqueue/orchestrator/controller/dlq/buildsignal_test.go b/submitqueue/orchestrator/controller/dlq/buildsignal_test.go index 39ae7df45..c573fa539 100644 --- a/submitqueue/orchestrator/controller/dlq/buildsignal_test.go +++ b/submitqueue/orchestrator/controller/dlq/buildsignal_test.go @@ -26,13 +26,14 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) func TestDLQBuildSignalController_InterfaceAndAccessors(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQBuildSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyBuildSignal), "orchestrator-buildsignal-dlq") @@ -70,7 +71,7 @@ func TestDLQBuildSignalController_Process_FansOutToBatch(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBuildStore().Return(buildStore).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -87,7 +88,7 @@ func TestDLQBuildSignalController_Process_FansOutToBatch(t *testing.T) { func TestDLQBuildSignalController_Process_TenantPayloadQueueMismatchAcks(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := NewDLQBuildSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyBuildSignal), "orchestrator-buildsignal-dlq") payload, err := sqmq.MarshalID(sqmq.TopicKeyBuildSignal, "build-1", "q") @@ -102,7 +103,7 @@ func TestDLQBuildSignalController_Process_BuildNotFoundIsNoOp(t *testing.T) { buildStore := storagemock.NewMockBuildStore(ctrl) buildStore.EXPECT().Get(gomock.Any(), "build-1").Return(entity.Build{}, storage.ErrNotFound) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBuildStore().Return(buildStore).AnyTimes() @@ -123,7 +124,7 @@ func TestDLQBuildSignalController_Process_BuildMissingBatchIsNoOp(t *testing.T) ID: "build-1", BatchID: "", Status: entity.BuildStatusRunning, }, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBuildStore().Return(buildStore).AnyTimes() @@ -139,7 +140,7 @@ func TestDLQBuildSignalController_Process_BuildMissingBatchIsNoOp(t *testing.T) func TestDLQBuildSignalController_Process_MalformedPayloadFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQBuildSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyBuildSignal), "orchestrator-buildsignal-dlq") diff --git a/submitqueue/orchestrator/controller/dlq/dlq.go b/submitqueue/orchestrator/controller/dlq/dlq.go index 47960177d..34e95980b 100644 --- a/submitqueue/orchestrator/controller/dlq/dlq.go +++ b/submitqueue/orchestrator/controller/dlq/dlq.go @@ -39,6 +39,8 @@ import ( "fmt" "strings" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber/submitqueue/platform/consumer" corebatch "github.com/uber/submitqueue/submitqueue/core/batch" requestcore "github.com/uber/submitqueue/submitqueue/core/request" @@ -142,7 +144,7 @@ func flattenDetail(prefix string, detail map[string]any, out map[string]string) // left in place: DLQ means the pipeline failed to converge, so we cannot // confirm the cancel completed cleanly. Writing Error is the honest signal and // keeps the request from being stuck in a non-terminal state forever. -func failRequest(ctx context.Context, store storage.Storage, registry consumer.TopicRegistry, logger *zap.SugaredLogger, requestID, lastError string, metadata map[string]string) error { +func failRequest(ctx context.Context, store orchstorage.Storage, registry consumer.TopicRegistry, logger *zap.SugaredLogger, requestID, lastError string, metadata map[string]string) error { res, err := requestcore.TerminateRequest(ctx, store, registry, requestID, entity.RequestStateError, lastError, metadata) if err != nil { return fmt.Errorf("dlq reconcile request %s failed: %w", requestID, err) @@ -188,7 +190,7 @@ func failRequest(ctx context.Context, store storage.Storage, registry consumer.T // on real progress — republishing to wake the queue, say — can then tell a // first reconcile from a redelivery of one already done, and avoid doing it // again forever. -func failBatch(ctx context.Context, store storage.Storage, registry consumer.TopicRegistry, logger *zap.SugaredLogger, batchID, lastError string, metadata map[string]string) (bool, error) { +func failBatch(ctx context.Context, store orchstorage.Storage, registry consumer.TopicRegistry, logger *zap.SugaredLogger, batchID, lastError string, metadata map[string]string) (bool, error) { batch, err := store.GetBatchStore().Get(ctx, batchID) if err != nil { if errors.Is(err, storage.ErrNotFound) { diff --git a/submitqueue/orchestrator/controller/dlq/dlq_test.go b/submitqueue/orchestrator/controller/dlq/dlq_test.go index a9d1ffb78..64d46c000 100644 --- a/submitqueue/orchestrator/controller/dlq/dlq_test.go +++ b/submitqueue/orchestrator/controller/dlq/dlq_test.go @@ -19,6 +19,8 @@ import ( "fmt" "testing" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/uber-go/tally" @@ -27,6 +29,7 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -34,10 +37,12 @@ import ( // newQueueBatchStateStore returns a QueueBatchStateStore mock that accepts any // membership-record write; these tests never list record buckets. // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} // noBatchAssociations answers the owning-batch lookup with nothing, i.e. no // batch ever enrolled the request. @@ -85,7 +90,7 @@ func TestFailRequest_TerminalStates(t *testing.T) { ID: "q/1", Queue: "q", Version: 5, State: tt.state, }, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() registry := consumer.TopicRegistry{} @@ -126,7 +131,7 @@ func TestFailRequest_CancellingTransitionsToError(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -151,7 +156,7 @@ func TestFailRequest_TransitionsToError(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -176,7 +181,7 @@ func TestFailRequest_LogPublishErrorPropagates(t *testing.T) { return fmt.Errorf("publish boom") }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -190,7 +195,7 @@ func TestFailRequest_NotFoundIsNoOp(t *testing.T) { requestStore := storagemock.NewMockRequestStore(ctrl) requestStore.EXPECT().Get(gomock.Any(), "q/1").Return(entity.Request{}, storage.ErrNotFound) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -204,7 +209,7 @@ func TestFailRequest_GenericGetErrorIsNonRetryable(t *testing.T) { requestStore := storagemock.NewMockRequestStore(ctrl) requestStore.EXPECT().Get(gomock.Any(), "q/1").Return(entity.Request{}, fmt.Errorf("boom")) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -242,7 +247,7 @@ func TestFailBatch_TransitionsAndFansOut(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -272,7 +277,7 @@ func TestFailBatch_FailedFansOutForRepair(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -290,7 +295,7 @@ func TestFailBatch_DifferentTerminalOutcomeSkipsFanOut(t *testing.T) { ID: "q/batch/1", Queue: "q", Contains: []string{"q/1"}, State: state, Version: 5, }, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -328,7 +333,7 @@ func TestFailBatch_CancellingTransitionsToFailed(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -343,7 +348,7 @@ func TestFailBatch_NotFoundIsNoOp(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batchStore.EXPECT().Get(gomock.Any(), "q/batch/1").Return(entity.Batch{}, storage.ErrNotFound) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() diff --git a/submitqueue/orchestrator/controller/dlq/landconflictsignal.go b/submitqueue/orchestrator/controller/dlq/landconflictsignal.go index 0811fbc37..d9bdddca4 100644 --- a/submitqueue/orchestrator/controller/dlq/landconflictsignal.go +++ b/submitqueue/orchestrator/controller/dlq/landconflictsignal.go @@ -18,12 +18,13 @@ import ( "context" "fmt" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" runwaymq "github.com/uber/submitqueue/api/runway/messagequeue" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" "github.com/uber/submitqueue/platform/consumer" "github.com/uber/submitqueue/platform/metrics" - "github.com/uber/submitqueue/submitqueue/extension/storage" "go.uber.org/zap" ) @@ -34,7 +35,7 @@ import ( type landConflictSignalController struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -48,7 +49,7 @@ var _ consumer.Controller = (*landConflictSignalController)(nil) func NewDLQLandConflictSignalController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -80,7 +81,7 @@ func (c *landConflictSignalController) Process(ctx context.Context, delivery con return nil } - store, err := c.stores.For(storage.Config{QueueName: result.GetQueueName()}) + store, err := c.stores.For(orchstorage.Config{QueueName: result.GetQueueName()}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. diff --git a/submitqueue/orchestrator/controller/dlq/landconflictsignal_test.go b/submitqueue/orchestrator/controller/dlq/landconflictsignal_test.go index 8ef296971..867d10486 100644 --- a/submitqueue/orchestrator/controller/dlq/landconflictsignal_test.go +++ b/submitqueue/orchestrator/controller/dlq/landconflictsignal_test.go @@ -25,13 +25,14 @@ import ( "github.com/uber/submitqueue/platform/consumer" "github.com/uber/submitqueue/submitqueue/entity" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) func TestDLQLandConflictSignalController_InterfaceAndAccessors(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQLandConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-landconflictsignal-dlq") @@ -55,7 +56,7 @@ func TestDLQLandConflictSignalController_Process_ReconcilesRequest(t *testing.T) return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -70,7 +71,7 @@ func TestDLQLandConflictSignalController_Process_ReconcilesRequest(t *testing.T) func TestDLQLandConflictSignalController_Process_TenantPayloadQueueMismatchAcks(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := NewDLQLandConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-landconflictsignal-dlq") payload, err := runwaymq.Marshal(&runwaymq.MergeResult{Id: "q/1", QueueName: "q", Outcome: runwaypb.Outcome_FAILED, Reason: "boom"}) @@ -82,7 +83,7 @@ func TestDLQLandConflictSignalController_Process_TenantPayloadQueueMismatchAcks( func TestDLQLandConflictSignalController_Process_MalformedPayloadFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQLandConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-landconflictsignal-dlq") diff --git a/submitqueue/orchestrator/controller/dlq/landsignal.go b/submitqueue/orchestrator/controller/dlq/landsignal.go index b5df62568..96deb0f96 100644 --- a/submitqueue/orchestrator/controller/dlq/landsignal.go +++ b/submitqueue/orchestrator/controller/dlq/landsignal.go @@ -18,12 +18,13 @@ import ( "context" "fmt" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" runwaymq "github.com/uber/submitqueue/api/runway/messagequeue" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" "github.com/uber/submitqueue/platform/consumer" "github.com/uber/submitqueue/platform/metrics" - "github.com/uber/submitqueue/submitqueue/extension/storage" "go.uber.org/zap" ) @@ -34,7 +35,7 @@ import ( type landSignalController struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -47,7 +48,7 @@ var _ consumer.Controller = (*landSignalController)(nil) func NewDLQLandSignalController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -79,7 +80,7 @@ func (c *landSignalController) Process(ctx context.Context, delivery consumer.De return nil } - store, err := c.stores.For(storage.Config{QueueName: result.GetQueueName()}) + store, err := c.stores.For(orchstorage.Config{QueueName: result.GetQueueName()}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. diff --git a/submitqueue/orchestrator/controller/dlq/landsignal_test.go b/submitqueue/orchestrator/controller/dlq/landsignal_test.go index c64cc064e..28a6157e0 100644 --- a/submitqueue/orchestrator/controller/dlq/landsignal_test.go +++ b/submitqueue/orchestrator/controller/dlq/landsignal_test.go @@ -25,13 +25,14 @@ import ( "github.com/uber/submitqueue/platform/consumer" "github.com/uber/submitqueue/submitqueue/entity" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) func TestDLQLandSignalController_InterfaceAndAccessors(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQLandSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-landsignal-dlq") @@ -65,7 +66,7 @@ func TestDLQLandSignalController_Process_ReconcilesBatch(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -81,7 +82,7 @@ func TestDLQLandSignalController_Process_ReconcilesBatch(t *testing.T) { func TestDLQLandSignalController_Process_TenantPayloadQueueMismatchAcks(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := NewDLQLandSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-landsignal-dlq") payload, err := runwaymq.Marshal(&runwaymq.MergeResult{Id: "q/batch/1", QueueName: "q", Outcome: runwaypb.Outcome_FAILED, Reason: "boom"}) @@ -93,7 +94,7 @@ func TestDLQLandSignalController_Process_TenantPayloadQueueMismatchAcks(t *testi func TestDLQLandSignalController_Process_MalformedPayloadFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() c := NewDLQLandSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-landsignal-dlq") diff --git a/submitqueue/orchestrator/controller/dlq/request.go b/submitqueue/orchestrator/controller/dlq/request.go index 153c55afc..d266fb485 100644 --- a/submitqueue/orchestrator/controller/dlq/request.go +++ b/submitqueue/orchestrator/controller/dlq/request.go @@ -19,6 +19,8 @@ import ( "errors" "fmt" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" "github.com/uber/submitqueue/platform/consumer" @@ -74,7 +76,7 @@ func DecodeRequestID(primary consumer.TopicKey) RequestIDDecoder { type requestController struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry decode RequestIDDecoder topicKey consumer.TopicKey @@ -90,7 +92,7 @@ var _ consumer.Controller = (*requestController)(nil) func NewDLQRequestController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, decode RequestIDDecoder, topicKey consumer.TopicKey, @@ -132,7 +134,7 @@ func (c *requestController) Process(ctx context.Context, delivery consumer.Deliv return fmt.Errorf("dlq payload decoded to empty request id") } - store, err := c.stores.For(storage.Config{QueueName: rid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: rid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -193,7 +195,7 @@ func (c *requestController) Process(ctx context.Context, delivery consumer.Deliv // mid-promotion and owns it, and any other request state means the claim never // landed. A request that has since disappeared is left to failRequest, which // reports the missing row. -func owningBatch(ctx context.Context, store storage.Storage, requestID string) (string, error) { +func owningBatch(ctx context.Context, store orchstorage.Storage, requestID string) (string, error) { batches, _, err := corebatch.FindByRequestID(ctx, store, requestID) if err != nil { return "", err diff --git a/submitqueue/orchestrator/controller/dlq/request_test.go b/submitqueue/orchestrator/controller/dlq/request_test.go index b130e7f39..559a40745 100644 --- a/submitqueue/orchestrator/controller/dlq/request_test.go +++ b/submitqueue/orchestrator/controller/dlq/request_test.go @@ -28,13 +28,14 @@ import ( "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) func TestDLQRequestController_InterfaceAndAccessors(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noBatchAssociations(ctrl)).AnyTimes() @@ -59,7 +60,7 @@ func TestDLQRequestController_Process_LandRequestPayload(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noBatchAssociations(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -75,7 +76,7 @@ func TestDLQRequestController_Process_LandRequestPayload(t *testing.T) { func TestDLQRequestController_Process_TenantPayloadQueueMismatchAcks(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := NewDLQRequestController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, DecodeRequestID(topickey.TopicKeyValidate), TopicKey(topickey.TopicKeyValidate), "orchestrator-validate-dlq") payload, err := sqmq.MarshalID(sqmq.TopicKeyValidate, "q/1", "q") @@ -98,7 +99,7 @@ func TestDLQRequestController_Process_CancelRequestPayload(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noBatchAssociations(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -127,7 +128,7 @@ func TestDLQRequestController_Process_RequestIDPayload(t *testing.T) { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noBatchAssociations(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -149,7 +150,7 @@ func TestDLQRequestController_Process_DifferentTerminalOutcomeSkips(t *testing.T ID: "q/1", Queue: "q", Version: 5, State: entity.RequestStateLanded, }, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noBatchAssociations(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() @@ -166,7 +167,7 @@ func TestDLQRequestController_Process_DifferentTerminalOutcomeSkips(t *testing.T func TestDLQRequestController_Process_MalformedPayloadFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noBatchAssociations(ctrl)).AnyTimes() // no store calls expected @@ -181,7 +182,7 @@ func TestDLQRequestController_Process_MalformedPayloadFails(t *testing.T) { func TestDLQRequestController_Process_EmptyIDFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(noBatchAssociations(ctrl)).AnyTimes() // no store calls expected @@ -249,7 +250,7 @@ func TestDLQRequestController_Process_SkipsRequestOwnedByLiveBatch(t *testing.T) ID: "q/batch/1", Queue: "q", Contains: []string{"q/1"}, State: state, Version: 1, }, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(associations).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -288,7 +289,7 @@ func TestDLQRequestController_Process_FailsWhenEveryBatchIsTerminal(t *testing.T registry := newTestLogRegistry(t, ctrl, 1, func(entity.RequestLog) error { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(associations).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -332,7 +333,7 @@ func TestDLQRequestController_Process_FailsWhenCreatingBatchNeverClaimed(t *test registry := newTestLogRegistry(t, ctrl, 1, func(entity.RequestLog) error { return nil }) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(associations).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -370,7 +371,7 @@ func TestDLQRequestController_Process_SkipsWhenCreatingBatchAlreadyClaimed(t *te ID: "q/batch/1", Queue: "q", Contains: []string{"q/1"}, State: entity.BatchStateCreating, Version: 1, }, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestBatchStore().Return(associations).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() diff --git a/submitqueue/orchestrator/controller/dlq/speculate.go b/submitqueue/orchestrator/controller/dlq/speculate.go index 705fca4a1..e0f570a49 100644 --- a/submitqueue/orchestrator/controller/dlq/speculate.go +++ b/submitqueue/orchestrator/controller/dlq/speculate.go @@ -18,6 +18,8 @@ import ( "context" "fmt" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/uber-go/tally" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" "github.com/uber/submitqueue/platform/consumer" @@ -27,7 +29,6 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" "go.uber.org/zap" ) @@ -51,7 +52,7 @@ import ( type speculateController struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -65,7 +66,7 @@ var _ consumer.Controller = (*speculateController)(nil) func NewDLQSpeculateController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -101,7 +102,7 @@ func (c *speculateController) Process(ctx context.Context, delivery consumer.Del return fmt.Errorf("dlq payload decoded to empty batch id") } - store, err := c.stores.For(storage.Config{QueueName: bid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: bid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -179,7 +180,7 @@ func (c *speculateController) blame(delivery consumer.Delivery, payloadBatchID s // run, so a queue that is genuinely broken drains to empty — every batch // recorded with a reason — instead of stranding, while a queue whose failure // was transient or queue-wide simply recovers on the next run. -func (c *speculateController) retrigger(ctx context.Context, store storage.Storage, queue string) error { +func (c *speculateController) retrigger(ctx context.Context, store orchstorage.Storage, queue string) error { const opName = "process" live, err := corebatch.ListByStates(ctx, store, entity.ActiveBatchStates()) diff --git a/submitqueue/orchestrator/controller/dlq/speculate_test.go b/submitqueue/orchestrator/controller/dlq/speculate_test.go index ddc62e12c..3c72345f2 100644 --- a/submitqueue/orchestrator/controller/dlq/speculate_test.go +++ b/submitqueue/orchestrator/controller/dlq/speculate_test.go @@ -28,11 +28,12 @@ import ( "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) -func newSpeculateController(registry consumer.TopicRegistry, store *storagemock.MockStorage, t *testing.T) consumer.Controller { +func newSpeculateController(registry consumer.TopicRegistry, store *orchstoragemock.MockStorage, t *testing.T) consumer.Controller { return NewDLQSpeculateController( zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, registry, TopicKey(topickey.TopicKeySpeculate), "orchestrator-speculate-dlq", @@ -139,7 +140,7 @@ func TestDLQSpeculateController_Process_Attribution(t *testing.T) { queueBatchState.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() queueBatchState.EXPECT().List(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() store.EXPECT().GetQueueBatchStateStore().Return(queueBatchState).AnyTimes() @@ -194,7 +195,7 @@ func TestDLQSpeculateController_Process_RetriggersQueue(t *testing.T) { }, nil).AnyTimes() queueBatchState.EXPECT().List(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetQueueBatchStateStore().Return(queueBatchState).AnyTimes() @@ -233,7 +234,7 @@ func TestDLQSpeculateController_Process_NoRetriggerWithoutProgress(t *testing.T) }, nil).AnyTimes() queueBatchState.EXPECT().List(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetQueueBatchStateStore().Return(queueBatchState).AnyTimes() @@ -252,7 +253,7 @@ func TestDLQSpeculateController_Process_NoRetriggerWithoutProgress(t *testing.T) func TestDLQSpeculateController_InterfaceAndAccessors(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := newSpeculateController(consumer.TopicRegistry{}, store, t) @@ -263,7 +264,7 @@ func TestDLQSpeculateController_InterfaceAndAccessors(t *testing.T) { func TestDLQSpeculateController_Process_MalformedPayloadFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := newSpeculateController(consumer.TopicRegistry{}, store, t) @@ -273,7 +274,7 @@ func TestDLQSpeculateController_Process_MalformedPayloadFails(t *testing.T) { func TestDLQSpeculateController_Process_TenantPayloadQueueMismatchAcks(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := newSpeculateController(consumer.TopicRegistry{}, store, t) payload, err := sqmq.MarshalID(sqmq.TopicKeySpeculate, "q/batch/named", "q") @@ -284,7 +285,7 @@ func TestDLQSpeculateController_Process_TenantPayloadQueueMismatchAcks(t *testin func TestDLQSpeculateController_Process_EmptyIDFails(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) c := newSpeculateController(consumer.TopicRegistry{}, store, t) diff --git a/submitqueue/orchestrator/controller/land/BUILD.bazel b/submitqueue/orchestrator/controller/land/BUILD.bazel index 502628526..e283781df 100644 --- a/submitqueue/orchestrator/controller/land/BUILD.bazel +++ b/submitqueue/orchestrator/controller/land/BUILD.bazel @@ -17,7 +17,7 @@ go_library( "//submitqueue/core/messagequeue:go_default_library", "//submitqueue/core/request:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -40,8 +40,9 @@ go_test( "//submitqueue/core/messagequeue:go_default_library", "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/land/land.go b/submitqueue/orchestrator/controller/land/land.go index 696cf72f3..9915242b9 100644 --- a/submitqueue/orchestrator/controller/land/land.go +++ b/submitqueue/orchestrator/controller/land/land.go @@ -39,7 +39,7 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" corerequest "github.com/uber/submitqueue/submitqueue/core/request" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // Controller handles land queue messages. Implements consumer.Controller. @@ -53,7 +53,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry runwayTopicKey consumer.TopicKey topicKey consumer.TopicKey @@ -69,7 +69,7 @@ var _ consumer.Controller = (*Controller)(nil) func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, runwayTopicKey consumer.TopicKey, topicKey consumer.TopicKey, @@ -107,7 +107,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: bid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: bid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -186,7 +186,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // buildLandRequest loads the batch's member requests and assembles the runway // MergeRequest: one MergeStep per request, in Contains order, attributed by // request id and carrying that request's change and land strategy. -func (c *Controller) buildLandRequest(ctx context.Context, store storage.Storage, batch entity.Batch) (*runwaymq.MergeRequest, error) { +func (c *Controller) buildLandRequest(ctx context.Context, store orchstorage.Storage, batch entity.Batch) (*runwaymq.MergeRequest, error) { steps := make([]*runwaymq.MergeStep, 0, len(batch.Contains)) for _, requestID := range batch.Contains { request, err := store.GetRequestStore().Get(ctx, requestID) diff --git a/submitqueue/orchestrator/controller/land/land_test.go b/submitqueue/orchestrator/controller/land/land_test.go index 2c05a02f3..ca8a69107 100644 --- a/submitqueue/orchestrator/controller/land/land_test.go +++ b/submitqueue/orchestrator/controller/land/land_test.go @@ -37,15 +37,18 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" ) // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} func batchIDPayload(t *testing.T, id, queue string) []byte { payload, err := sqmq.MarshalID(sqmq.TopicKeyLand, id, queue) @@ -82,7 +85,7 @@ func newDelivery(t *testing.T, ctrl *gomock.Controller, batchID, partitionKey st return delivery } -func newController(t *testing.T, store *storagemock.MockStorage, registry consumer.TopicRegistry) *Controller { +func newController(t *testing.T, store *orchstoragemock.MockStorage, registry consumer.TopicRegistry) *Controller { return NewController( zaptest.NewLogger(t).Sugar(), tally.NoopScope, @@ -133,7 +136,7 @@ func newRegistry(t *testing.T, ctrl *gomock.Controller, failTopic string) (consu func TestNewController(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) q := queuemock.NewMockQueue(ctrl) registry, err := consumer.NewTopicRegistry( []consumer.TopicConfig{{Key: runwaymq.TopicKeyMerge, Name: "runway-merge", Queue: q}}, @@ -151,7 +154,7 @@ func TestNewController(t *testing.T) { func TestProcess_RejectsTenantPayloadQueueMismatch(t *testing.T) { ctrl := gomock.NewController(t) - c := newController(t, storagemock.NewMockStorage(ctrl), consumer.TopicRegistry{}) + c := newController(t, orchstoragemock.NewMockStorage(ctrl), consumer.TopicRegistry{}) msg := entityqueue.NewMessage("test-queue/batch/1", batchIDPayload(t, "test-queue/batch/1", "test-queue"), "test-queue", nil) msg.Tenant = "other-queue" delivery := consumermock.NewMockDelivery(ctrl) @@ -190,7 +193,7 @@ func TestProcess_PublishesFullPayloadToRunway(t *testing.T) { reqStore.EXPECT().Get(gomock.Any(), req1.ID).Return(req1, nil) reqStore.EXPECT().Get(gomock.Any(), req2.ID).Return(req2, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() @@ -257,7 +260,7 @@ func TestProcess_HaltedBatchSkips(t *testing.T) { // No request-store reads and no publish for a halted batch: the // members are told nothing and runway is not asked to land. - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() registry, rec := newRegistry(t, ctrl, "") @@ -292,7 +295,7 @@ func TestProcess_ReportsLandingBeforeDispatch(t *testing.T) { reqStore.EXPECT().Get(gomock.Any(), req1.ID).Return(req1, nil) reqStore.EXPECT().Get(gomock.Any(), req2.ID).Return(req2, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() @@ -340,7 +343,7 @@ func TestProcess_PublishFailureReturnsError(t *testing.T) { reqStore := storagemock.NewMockRequestStore(ctrl) reqStore.EXPECT().Get(gomock.Any(), req.ID).Return(req, nil).AnyTimes() - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() @@ -366,7 +369,7 @@ func TestProcess_BatchStoreGetFailureNotRetryable(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batchStore.EXPECT().Get(gomock.Any(), batchID).Return(entity.Batch{}, fmt.Errorf("db connection lost")) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() q := queuemock.NewMockQueue(ctrl) diff --git a/submitqueue/orchestrator/controller/landconflictsignal/BUILD.bazel b/submitqueue/orchestrator/controller/landconflictsignal/BUILD.bazel index 589c316ae..164073be4 100644 --- a/submitqueue/orchestrator/controller/landconflictsignal/BUILD.bazel +++ b/submitqueue/orchestrator/controller/landconflictsignal/BUILD.bazel @@ -16,7 +16,7 @@ go_library( "//submitqueue/core/request:go_default_library", "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -36,8 +36,9 @@ go_test( "//submitqueue/core/messagequeue:go_default_library", "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal.go b/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal.go index 46178bbf4..22106f2f2 100644 --- a/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal.go +++ b/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal.go @@ -34,7 +34,7 @@ import ( corerequest "github.com/uber/submitqueue/submitqueue/core/request" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -42,7 +42,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -55,7 +55,7 @@ var _ consumer.Controller = (*Controller)(nil) func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -94,7 +94,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: result.GetQueueName()}) + store, err := c.stores.For(orchstorage.Config{QueueName: result.GetQueueName()}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -173,7 +173,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // in Error skips the state CAS but still publishes the log (so a prior attempt // that flipped the state but failed before logging is repaired); a request that // reached a different terminal state (e.g. a racing cancel) is left untouched. -func (c *Controller) failRequest(ctx context.Context, store storage.Storage, request entity.Request, reason string) error { +func (c *Controller) failRequest(ctx context.Context, store orchstorage.Storage, request entity.Request, reason string) error { switch { case request.State == entity.RequestStateError: // Idempotent retry: a prior delivery already wrote Error. Fall through to diff --git a/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal_test.go b/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal_test.go index 64093cbfb..f51e273da 100644 --- a/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal_test.go +++ b/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal_test.go @@ -31,17 +31,20 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} func requestWithState(request entity.Request, state entity.RequestState) entity.Request { request.State = state @@ -72,7 +75,7 @@ const ( func TestProcess_RejectsTenantPayloadQueueMismatch(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) controller := NewController(zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: store}, consumer.TopicRegistry{}, runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-landconflictsignal") res := runwaymq.MergeResult{Id: testRequestID, Outcome: runwaypb.Outcome_SUCCEEDED} @@ -92,7 +95,7 @@ func TestProcess_LandablePublishesToBatch(t *testing.T) { reqStore.EXPECT().Get(gomock.Any(), testRequestID).Return(request, nil) reqStore.EXPECT().Update(gomock.Any(), requestWithState(request, entity.RequestStateValidated), int32(1), int32(2)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() var gotTopics []string @@ -147,7 +150,7 @@ func TestProcess_NotLandableMarksRequestError(t *testing.T) { // The request is driven to terminal Error inline (version 1 -> 2). reqStore.EXPECT().Update(gomock.Any(), requestWithState(request, entity.RequestStateError), int32(1), int32(2)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() // One publish is expected — the terminal log entry to the log topic. A publish @@ -196,7 +199,7 @@ func TestFailRequest_UpdateFailureLeavesRequestUnchanged(t *testing.T) { reqStore := storagemock.NewMockRequestStore(ctrl) reqStore.EXPECT().Update(gomock.Any(), requestWithState(request, entity.RequestStateError), int32(1), int32(2)).Return(fmt.Errorf("db down")) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(reqStore) controller := NewController(zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: store}, consumer.TopicRegistry{}, @@ -215,7 +218,7 @@ func TestProcess_HaltedRequestSkips(t *testing.T) { reqStore.EXPECT().Get(gomock.Any(), testRequestID).Return( entity.Request{ID: testRequestID, Queue: testQueue, State: entity.RequestStateCancelled, Version: 4}, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(reqStore).AnyTimes() // No publish: gomock fails if a batch publish runs for a halted request. diff --git a/submitqueue/orchestrator/controller/landsignal/BUILD.bazel b/submitqueue/orchestrator/controller/landsignal/BUILD.bazel index 855374dab..a0612baa0 100644 --- a/submitqueue/orchestrator/controller/landsignal/BUILD.bazel +++ b/submitqueue/orchestrator/controller/landsignal/BUILD.bazel @@ -16,7 +16,7 @@ go_library( "//submitqueue/core/messagequeue:go_default_library", "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -35,8 +35,9 @@ go_test( "//platform/extension/messagequeue/mock:go_default_library", "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", - "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/landsignal/landsignal.go b/submitqueue/orchestrator/controller/landsignal/landsignal.go index efc52365d..f61934dea 100644 --- a/submitqueue/orchestrator/controller/landsignal/landsignal.go +++ b/submitqueue/orchestrator/controller/landsignal/landsignal.go @@ -36,7 +36,7 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -44,7 +44,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -57,7 +57,7 @@ var _ consumer.Controller = (*Controller)(nil) func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -96,7 +96,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: result.GetQueueName()}) + store, err := c.stores.For(orchstorage.Config{QueueName: result.GetQueueName()}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. diff --git a/submitqueue/orchestrator/controller/landsignal/landsignal_test.go b/submitqueue/orchestrator/controller/landsignal/landsignal_test.go index fafac56b1..c713531e2 100644 --- a/submitqueue/orchestrator/controller/landsignal/landsignal_test.go +++ b/submitqueue/orchestrator/controller/landsignal/landsignal_test.go @@ -29,8 +29,9 @@ import ( queuemock "github.com/uber/submitqueue/platform/extension/messagequeue/mock" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -38,10 +39,12 @@ import ( // newQueueBatchStateStore returns a QueueBatchStateStore mock that accepts any // membership-record write; these tests never list record buckets. // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} func newQueueBatchStateStore(ctrl *gomock.Controller) *storagemock.MockQueueBatchStateStore { s := storagemock.NewMockQueueBatchStateStore(ctrl) @@ -98,7 +101,7 @@ func recordingRegistry(t *testing.T, ctrl *gomock.Controller, got *[]string) con return registry } -func newController(t *testing.T, store *storagemock.MockStorage, registry consumer.TopicRegistry) *Controller { +func newController(t *testing.T, store *orchstoragemock.MockStorage, registry consumer.TopicRegistry) *Controller { return NewController( zaptest.NewLogger(t).Sugar(), tally.NoopScope, @@ -111,7 +114,7 @@ func newController(t *testing.T, store *storagemock.MockStorage, registry consum func TestNewController(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() var got []string c := newController(t, store, recordingRegistry(t, ctrl, &got)) @@ -124,7 +127,7 @@ func TestNewController(t *testing.T) { func TestProcess_RejectsTenantPayloadQueueMismatch(t *testing.T) { ctrl := gomock.NewController(t) - c := newController(t, storagemock.NewMockStorage(ctrl), recordingRegistry(t, ctrl, new([]string))) + c := newController(t, orchstoragemock.NewMockStorage(ctrl), recordingRegistry(t, ctrl, new([]string))) res := runwaymq.MergeResult{Id: testBatchID, Outcome: runwaypb.Outcome_SUCCEEDED} msg := entityqueue.NewMessage(testBatchID, resultPayload(t, res), testQueue, nil) msg.Tenant = "other-queue" @@ -149,7 +152,7 @@ func TestProcess_LandedAdvancesBatch(t *testing.T) { batchStore.EXPECT().Get(gomock.Any(), testBatchID).Return(batch, nil) batchStore.EXPECT().Update(gomock.Any(), batchWithState(batch, entity.BatchStateSucceeded), int32(1), int32(2)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -192,7 +195,7 @@ func TestProcess_FanoutDoesNotCollideWithTheBatchAnnouncement(t *testing.T) { batchStore.EXPECT().Get(gomock.Any(), testBatchID).Return(batch, nil) batchStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -239,7 +242,7 @@ func TestProcess_NotLandedMarksBatchFailed(t *testing.T) { // to conclude on the message, not on the batch. batchStore.EXPECT().Update(gomock.Any(), batchWithState(batch, entity.BatchStateFailed), int32(3), int32(4)).Return(nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -279,7 +282,7 @@ func TestProcess_CancellingShortCircuit(t *testing.T) { batchStore.EXPECT().Get(gomock.Any(), testBatchID).Return( entity.Batch{ID: testBatchID, Queue: testQueue, State: entity.BatchStateCancelling, Version: 4}, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -302,7 +305,7 @@ func TestProcess_TerminalReFansOut(t *testing.T) { batchStore.EXPECT().Get(gomock.Any(), testBatchID).Return( entity.Batch{ID: testBatchID, Queue: testQueue, State: entity.BatchStateSucceeded, Version: 5}, nil) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -318,7 +321,7 @@ func TestProcess_TerminalReFansOut(t *testing.T) { func TestProcess_DeserializeErrorRejects(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() var got []string c := newController(t, store, recordingRegistry(t, ctrl, &got)) @@ -333,7 +336,7 @@ func TestProcess_StorageErrorRejects(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batchStore.EXPECT().Get(gomock.Any(), testBatchID).Return(entity.Batch{}, assert.AnError) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() diff --git a/submitqueue/orchestrator/controller/speculate/BUILD.bazel b/submitqueue/orchestrator/controller/speculate/BUILD.bazel index 0d9ada4c3..e60b0047b 100644 --- a/submitqueue/orchestrator/controller/speculate/BUILD.bazel +++ b/submitqueue/orchestrator/controller/speculate/BUILD.bazel @@ -29,6 +29,7 @@ go_library( "//submitqueue/entity:go_default_library", "//submitqueue/extension/speculation/speculator:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -55,6 +56,8 @@ go_test( "//submitqueue/extension/speculation/speculator:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/speculate/dispatch.go b/submitqueue/orchestrator/controller/speculate/dispatch.go index 5e3883f87..c7ce00f36 100644 --- a/submitqueue/orchestrator/controller/speculate/dispatch.go +++ b/submitqueue/orchestrator/controller/speculate/dispatch.go @@ -24,7 +24,8 @@ import ( "github.com/uber/submitqueue/platform/publish" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // dispatch saves what finalize left over and hands the build stage its work. @@ -120,7 +121,7 @@ func (c *Controller) dispatch(ctx context.Context, queue string, snap snapshot, // persist writes a head's path set, creating it if this run is the first to // fund the head. It returns the set as stored, with its version advanced, so // a caller that keeps the set around goes on holding a current copy. -func (c *Controller) persist(ctx context.Context, store storage.Storage, set entity.SpeculationPathSet, exists bool) (entity.SpeculationPathSet, error) { +func (c *Controller) persist(ctx context.Context, store orchstorage.Storage, set entity.SpeculationPathSet, exists bool) (entity.SpeculationPathSet, error) { pathSets := store.GetSpeculationPathSetStore() if !exists { diff --git a/submitqueue/orchestrator/controller/speculate/finalize.go b/submitqueue/orchestrator/controller/speculate/finalize.go index 0b08f8fa3..b160c0400 100644 --- a/submitqueue/orchestrator/controller/speculate/finalize.go +++ b/submitqueue/orchestrator/controller/speculate/finalize.go @@ -26,7 +26,8 @@ import ( corerequest "github.com/uber/submitqueue/submitqueue/core/request" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // finalize reaches and enacts every outcome this run can conclude, and leaves @@ -303,7 +304,7 @@ func (c *Controller) recordOutcome(snap *snapshot, batchID string, decision outc // consumer can act on an outcome a lost compare-and-swap refused to write. The // cost is a dispatch that fails on a batch finalize no longer walks, which the // recovery message and Process's self-heal exist to repair. -func (c *Controller) applyOutcome(ctx context.Context, store storage.Storage, batch entity.Batch, decision outcome, isTriggerBatch bool) (bool, error) { +func (c *Controller) applyOutcome(ctx context.Context, store orchstorage.Storage, batch entity.Batch, decision outcome, isTriggerBatch bool) (bool, error) { var state entity.BatchState switch decision { @@ -413,7 +414,7 @@ func (c *Controller) dispatchLand(ctx context.Context, batch entity.Batch) error // decision from unchanged state. A duplicate is harmless — Speculate // tolerates any batch state, and if the write never lands the message is just // a nudge that re-plans a queue nothing has changed. -func (c *Controller) recoverable(ctx context.Context, store storage.Storage, batch entity.Batch) error { +func (c *Controller) recoverable(ctx context.Context, store orchstorage.Storage, batch entity.Batch) error { if err := c.publishBatchID(ctx, topickey.TopicKeySpeculate, publish.UniqueID(batch.ID), batch.ID, batch.Queue, batch.Queue); err != nil { metrics.NamedCounter(c.metricsScope, opName, "publish_errors", 1) return fmt.Errorf("failed to publish recovery signal for batch %s: %w", batch.ID, err) diff --git a/submitqueue/orchestrator/controller/speculate/run.go b/submitqueue/orchestrator/controller/speculate/run.go index dba882c63..33ec469f0 100644 --- a/submitqueue/orchestrator/controller/speculate/run.go +++ b/submitqueue/orchestrator/controller/speculate/run.go @@ -25,7 +25,8 @@ import ( corebatch "github.com/uber/submitqueue/submitqueue/core/batch" "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // run re-plans a whole queue from a single read of its state, in the six @@ -34,7 +35,7 @@ import ( // The batch on the triggering message only says which queue woke up; nothing // about the plan depends on which batch it was, or on any earlier run. Its // identity is carried through only for crash recovery — see snapshot.trigger. -func (c *Controller) run(ctx context.Context, store storage.Storage, trigger entity.Batch) error { +func (c *Controller) run(ctx context.Context, store orchstorage.Storage, trigger entity.Batch) error { snap, err := c.read(ctx, store, trigger.Queue) if err != nil { return err @@ -114,7 +115,7 @@ func (c *Controller) admitCreated(ctx context.Context, snap *snapshot) { // read builds the run's snapshot. Batches come first because their dependency // lists say which finalized batches still have to be resolved, and their IDs // say which path sets to load. -func (c *Controller) read(ctx context.Context, store storage.Storage, queue string) (snapshot, error) { +func (c *Controller) read(ctx context.Context, store orchstorage.Storage, queue string) (snapshot, error) { inFlight, err := corebatch.ListByStates(ctx, store, entity.ActiveBatchStates()) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_errors", 1) @@ -193,7 +194,7 @@ func (c *Controller) read(ctx context.Context, store storage.Storage, queue stri // the run makes later, which keeps this controller the path set's single // writer: the build stages record what CI did on per-build records, and the // run alone decides when that becomes the path's status. -func (c *Controller) updatePathsFromBuilds(ctx context.Context, store storage.Storage, set *entity.SpeculationPathSet) (bool, error) { +func (c *Controller) updatePathsFromBuilds(ctx context.Context, store orchstorage.Storage, set *entity.SpeculationPathSet) (bool, error) { changed := false for i := range set.Paths { entry := &set.Paths[i] diff --git a/submitqueue/orchestrator/controller/speculate/run_test.go b/submitqueue/orchestrator/controller/speculate/run_test.go index 32597f238..6bf60c1d2 100644 --- a/submitqueue/orchestrator/controller/speculate/run_test.go +++ b/submitqueue/orchestrator/controller/speculate/run_test.go @@ -31,6 +31,7 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -70,7 +71,7 @@ func (m updateTo) String() string { type runHarness struct { controller *Controller - store *storagemock.MockStorage + store *orchstoragemock.MockStorage batches *storagemock.MockBatchStore pathSets *storagemock.MockSpeculationPathSetStore pathBuilds *storagemock.MockPathBuildStore @@ -158,7 +159,7 @@ func newRunHarness(t *testing.T, ctrl *gomock.Controller, spec *scriptedSpeculat h.pathBuilds = storagemock.NewMockPathBuildStore(ctrl) h.builds = storagemock.NewMockBuildStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) h.store = store store.EXPECT().GetQueueBatchStateStore().Return(queueStates).AnyTimes() store.EXPECT().GetBatchStore().Return(h.batches).AnyTimes() @@ -1101,7 +1102,7 @@ func TestFanout_MintsADistinctMessageIDPerPublish(t *testing.T) { require.NoError(t, err) c := NewController( - zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: storagemock.NewMockStorage(ctrl)}, + zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: orchstoragemock.NewMockStorage(ctrl)}, staticSpeculatorFactory{}, registry, topickey.TopicKeySpeculate, "orchestrator-speculate", ) @@ -1134,7 +1135,7 @@ func TestDispatchLand_ReusesOneMessageIDPerBatch(t *testing.T) { require.NoError(t, err) c := NewController( - zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: storagemock.NewMockStorage(ctrl)}, + zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: orchstoragemock.NewMockStorage(ctrl)}, staticSpeculatorFactory{}, registry, topickey.TopicKeySpeculate, "orchestrator-speculate", ) batch := entity.Batch{ID: head, Queue: "q"} diff --git a/submitqueue/orchestrator/controller/speculate/snapshot.go b/submitqueue/orchestrator/controller/speculate/snapshot.go index a1677d12d..31781b981 100644 --- a/submitqueue/orchestrator/controller/speculate/snapshot.go +++ b/submitqueue/orchestrator/controller/speculate/snapshot.go @@ -16,7 +16,7 @@ package speculate import ( "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // snapshot is one run's working state: the queue as it was read, plus the @@ -27,7 +27,7 @@ import ( type snapshot struct { // store is the queue-scoped store aggregate this run reads and writes // through, resolved once from the triggering message's queue. - store storage.Storage + store orchstorage.Storage // batches is every batch the run can reason about, by ID: the queue's // in-flight batches plus any finalized batch still named as a dependency // of one of them. diff --git a/submitqueue/orchestrator/controller/speculate/speculate.go b/submitqueue/orchestrator/controller/speculate/speculate.go index fb6ed0fef..053a885d3 100644 --- a/submitqueue/orchestrator/controller/speculate/speculate.go +++ b/submitqueue/orchestrator/controller/speculate/speculate.go @@ -31,7 +31,7 @@ import ( "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator" - "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -41,7 +41,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory speculators speculator.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey @@ -58,7 +58,7 @@ const opName = "process" func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, speculators speculator.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, @@ -105,7 +105,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: bid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: bid.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -167,7 +167,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // in between re-publishes under the same occurrence and dedupes. The cost is // that a transition which then loses its compare-and-swap leaves one entry for // a batch that never speculated, superseded by whatever the winner wrote. -func (c *Controller) admit(ctx context.Context, store storage.Storage, batch entity.Batch) (entity.Batch, error) { +func (c *Controller) admit(ctx context.Context, store orchstorage.Storage, batch entity.Batch) (entity.Batch, error) { if err := corerequest.PublishBatchLogs(ctx, c.registry, batch.Queue, batch.Contains, entity.RequestStatusSpeculating, batch.ID, map[string]string{"batch_id": batch.ID}, ); err != nil { diff --git a/submitqueue/orchestrator/controller/speculate/speculate_test.go b/submitqueue/orchestrator/controller/speculate/speculate_test.go index 020e82628..5855bf7d2 100644 --- a/submitqueue/orchestrator/controller/speculate/speculate_test.go +++ b/submitqueue/orchestrator/controller/speculate/speculate_test.go @@ -19,6 +19,8 @@ import ( "fmt" "testing" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/uber-go/tally" @@ -32,6 +34,7 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator" "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) @@ -56,10 +59,12 @@ func (f staticSpeculatorFactory) For(speculator.Config) (speculator.Speculator, } // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} // listsInFlight makes the queue read return exactly these batches: each is // filed under its state's membership bucket and hydrated back through the @@ -128,7 +133,7 @@ func newProcHarness(t *testing.T, ctrl *gomock.Controller, publishErr error) *pr h.pathBuilds = storagemock.NewMockPathBuildStore(ctrl) h.builds = storagemock.NewMockBuildStore(ctrl) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(h.batches).AnyTimes() store.EXPECT().GetQueueBatchStateStore().Return(h.queueStates).AnyTimes() store.EXPECT().GetSpeculationPathSetStore().Return(h.pathSets).AnyTimes() diff --git a/submitqueue/orchestrator/controller/start/BUILD.bazel b/submitqueue/orchestrator/controller/start/BUILD.bazel index 3038163b2..5ad556126 100644 --- a/submitqueue/orchestrator/controller/start/BUILD.bazel +++ b/submitqueue/orchestrator/controller/start/BUILD.bazel @@ -15,6 +15,7 @@ go_library( "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -37,6 +38,8 @@ go_test( "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/storage/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/start/start.go b/submitqueue/orchestrator/controller/start/start.go index 8d3a1f7dc..ac1a22b08 100644 --- a/submitqueue/orchestrator/controller/start/start.go +++ b/submitqueue/orchestrator/controller/start/start.go @@ -28,7 +28,8 @@ import ( corerequest "github.com/uber/submitqueue/submitqueue/core/request" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -41,7 +42,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry topicKey consumer.TopicKey consumerGroup string @@ -54,7 +55,7 @@ var _ consumer.Controller = (*Controller)(nil) func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string, @@ -87,7 +88,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: landRequest.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: landRequest.Queue}) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. diff --git a/submitqueue/orchestrator/controller/start/start_test.go b/submitqueue/orchestrator/controller/start/start_test.go index 196f377cb..461c634ce 100644 --- a/submitqueue/orchestrator/controller/start/start_test.go +++ b/submitqueue/orchestrator/controller/start/start_test.go @@ -32,23 +32,27 @@ import ( sqmq "github.com/uber/submitqueue/submitqueue/core/messagequeue" "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) // newTestController creates a controller with test dependencies. // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} func newTestController( t *testing.T, ctrl *gomock.Controller, - store *storagemock.MockStorage, + store *orchstoragemock.MockStorage, publishErr error, ) *Controller { logger := zaptest.NewLogger(t).Sugar() @@ -76,11 +80,11 @@ func newTestController( } // newMockStorage creates a MockStorage with a MockRequestStore that succeeds on Create. -func newMockStorage(ctrl *gomock.Controller) *storagemock.MockStorage { +func newMockStorage(ctrl *gomock.Controller) *orchstoragemock.MockStorage { mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() return store } @@ -165,7 +169,7 @@ func TestController_Process_ConstructsRequestWithStateAndVersion(t *testing.T) { return nil }, ) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() controller := newTestController(t, ctrl, store, nil) @@ -234,7 +238,7 @@ func TestController_Process_StorageFailure(t *testing.T) { mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(fmt.Errorf("database connection failed")) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() controller := newTestController(t, ctrl, store, nil) @@ -256,7 +260,7 @@ func TestController_Process_AlreadyExistsSucceeds(t *testing.T) { mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(fmt.Errorf("duplicate: %w", storage.ErrAlreadyExists)) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() controller := newTestController(t, ctrl, store, nil) diff --git a/submitqueue/orchestrator/controller/validate/BUILD.bazel b/submitqueue/orchestrator/controller/validate/BUILD.bazel index c07f3074f..cfa3fe738 100644 --- a/submitqueue/orchestrator/controller/validate/BUILD.bazel +++ b/submitqueue/orchestrator/controller/validate/BUILD.bazel @@ -20,6 +20,7 @@ go_library( "//submitqueue/extension/changeprovider:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/extension/validator:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_uber_go_tally//:go_default_library", "@org_uber_go_zap//:go_default_library", ], @@ -47,6 +48,8 @@ go_test( "//submitqueue/extension/storage/mock:go_default_library", "//submitqueue/extension/validator:go_default_library", "//submitqueue/extension/validator/mock:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", "@com_github_uber_go_tally//:go_default_library", diff --git a/submitqueue/orchestrator/controller/validate/validate.go b/submitqueue/orchestrator/controller/validate/validate.go index d8f3313f1..657f1c5c8 100644 --- a/submitqueue/orchestrator/controller/validate/validate.go +++ b/submitqueue/orchestrator/controller/validate/validate.go @@ -33,8 +33,9 @@ import ( corerequest "github.com/uber/submitqueue/submitqueue/core/request" "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/changeprovider" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" "github.com/uber/submitqueue/submitqueue/extension/validator" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -46,7 +47,7 @@ import ( type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope - stores storage.Factory + stores orchstorage.Factory registry consumer.TopicRegistry changeProviders changeprovider.Factory validators validator.Factory @@ -65,7 +66,7 @@ var _ consumer.Controller = (*Controller)(nil) func NewController( logger *zap.SugaredLogger, scope tally.Scope, - stores storage.Factory, + stores orchstorage.Factory, registry consumer.TopicRegistry, changeProviders changeprovider.Factory, validators validator.Factory, @@ -102,7 +103,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("invalid message identity: %w", err) } - store, err := c.stores.For(storage.Config{QueueName: rid.Queue}) + store, err := c.stores.For(orchstorage.Config{QueueName: rid.Queue}) if err != nil { coremetrics.NamedCounter(c.metricsScope, "process", "storage_resolve_errors", 1) // Non-retryable: a missing or unresolvable queue is a malformed message. @@ -261,7 +262,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // Only an infra failure while terminating (storage/publish) is returned as an // error so the delivery is retried; the request itself is never re-queued for // validation once rejected. -func (c *Controller) reject(ctx context.Context, store storage.Storage, requestID, reason string) error { +func (c *Controller) reject(ctx context.Context, store orchstorage.Storage, requestID, reason string) error { if _, err := corerequest.TerminateRequest(ctx, store, c.registry, requestID, entity.RequestStateError, reason, nil); err != nil { coremetrics.NamedCounter(c.metricsScope, "process", "terminate_errors", 1) return fmt.Errorf("failed to terminate rejected request %s: %w", requestID, err) @@ -280,7 +281,7 @@ func (c *Controller) reject(ctx context.Context, store storage.Storage, requestI // // Per-URI / per-record reads keep the contract backend-agnostic; the typical request // has 1-5 URIs, so the loop is cheap. -func (c *Controller) checkDuplicate(ctx context.Context, store storage.Storage, request entity.Request) (string, error) { +func (c *Controller) checkDuplicate(ctx context.Context, store orchstorage.Storage, request entity.Request) (string, error) { seenOwners := make(map[string]struct{}) for _, uri := range request.Change.URIs { records, err := store.GetChangeStore().GetByURI(ctx, uri) @@ -360,7 +361,7 @@ func toProtoStrategy(s mergestrategy.MergeStrategy) mergestrategypb.Strategy { // and its Details are written together in a single immutable Create — there is no // later mutation. Create is idempotent on its primary key, so a redelivery (or a // prior partial attempt) is a no-op and the first write wins. -func (c *Controller) claimChanges(ctx context.Context, store storage.Storage, request entity.Request, infos []entity.ChangeInfo) error { +func (c *Controller) claimChanges(ctx context.Context, store orchstorage.Storage, request entity.Request, infos []entity.ChangeInfo) error { now := time.Now().UnixMilli() for _, info := range infos { record := entity.ChangeRecord{ diff --git a/submitqueue/orchestrator/controller/validate/validate_test.go b/submitqueue/orchestrator/controller/validate/validate_test.go index 3fb384578..1cdcd95d0 100644 --- a/submitqueue/orchestrator/controller/validate/validate_test.go +++ b/submitqueue/orchestrator/controller/validate/validate_test.go @@ -35,19 +35,23 @@ import ( "github.com/uber/submitqueue/submitqueue/core/topickey" "github.com/uber/submitqueue/submitqueue/entity" changeprovidermock "github.com/uber/submitqueue/submitqueue/extension/changeprovider/mock" - "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/extension/storage" storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" "github.com/uber/submitqueue/submitqueue/extension/validator" validatormock "github.com/uber/submitqueue/submitqueue/extension/validator/mock" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + orchstoragemock "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" ) // staticStorageFactory resolves every queue to one fixed store aggregate. -type staticStorageFactory struct{ store storage.Storage } +type staticStorageFactory struct{ store orchstorage.Storage } // For returns the fixed store aggregate for any queue. -func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func (f staticStorageFactory) For(orchstorage.Config) (orchstorage.Storage, error) { + return f.store, nil +} func TestToProtoStrategy(t *testing.T) { tests := []struct { @@ -103,11 +107,11 @@ func (m *mockChangeProvider) Get(ctx context.Context, request entity.Request) ([ // newMockStorage creates a MockStorage with a MockRequestStore that returns the given request on Get. // The returned MockRequestStore is exposed so individual tests can layer additional Get expectations. -func newMockStorage(ctrl *gomock.Controller, request entity.Request) (*storagemock.MockStorage, *storagemock.MockRequestStore) { +func newMockStorage(ctrl *gomock.Controller, request entity.Request) (*orchstoragemock.MockStorage, *storagemock.MockRequestStore) { mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Get(gomock.Any(), request.ID).Return(request, nil).AnyTimes() - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() return store, mockReqStore } @@ -126,7 +130,7 @@ func newMockChangeStore(ctrl *gomock.Controller) *storagemock.MockChangeStore { func newTestController( t *testing.T, ctrl *gomock.Controller, - store *storagemock.MockStorage, + store *orchstoragemock.MockStorage, cs *storagemock.MockChangeStore, publishErr error, ) *Controller { @@ -204,7 +208,7 @@ func TestController_Process_Success(t *testing.T) { func TestController_Process_RejectsTenantPayloadQueueMismatch(t *testing.T) { ctrl := gomock.NewController(t) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) controller := newTestController(t, ctrl, store, storagemock.NewMockChangeStore(ctrl), nil) msg := entityqueue.NewMessage("test-queue/123", requestIDPayload(t, "test-queue/123"), "test-queue", nil) msg.Tenant = "other-queue" @@ -337,7 +341,7 @@ func TestController_Process_StorageFailure(t *testing.T) { mockReqStore := storagemock.NewMockRequestStore(ctrl) mockReqStore.EXPECT().Get(gomock.Any(), "test-queue/123").Return(entity.Request{}, fmt.Errorf("db connection lost")) - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() controller := newTestController(t, ctrl, store, newMockChangeStore(ctrl), nil) @@ -532,7 +536,7 @@ func TestController_Process_DuplicateDetection(t *testing.T) { // dead-lettered, then the delivery is acked. mockReqStore.EXPECT().Update(gomock.Any(), requestWithState(request, entity.RequestStateError), int32(1), int32(2)).Return(nil) } - store := storagemock.NewMockStorage(ctrl) + store := orchstoragemock.NewMockStorage(ctrl) store.EXPECT().GetRequestStore().Return(mockReqStore).AnyTimes() cs := storagemock.NewMockChangeStore(ctrl) diff --git a/submitqueue/orchestrator/extension/storage/BUILD.bazel b/submitqueue/orchestrator/extension/storage/BUILD.bazel new file mode 100644 index 000000000..46bb3e3d8 --- /dev/null +++ b/submitqueue/orchestrator/extension/storage/BUILD.bazel @@ -0,0 +1,9 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["storage.go"], + importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage", + visibility = ["//visibility:public"], + deps = ["//submitqueue/extension/storage:go_default_library"], +) diff --git a/submitqueue/orchestrator/extension/storage/mock/BUILD.bazel b/submitqueue/orchestrator/extension/storage/mock/BUILD.bazel new file mode 100644 index 000000000..e048a040b --- /dev/null +++ b/submitqueue/orchestrator/extension/storage/mock/BUILD.bazel @@ -0,0 +1,13 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["storage_mock.go"], + importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mock", + visibility = ["//visibility:public"], + deps = [ + "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "@org_uber_go_mock//gomock:go_default_library", + ], +) diff --git a/submitqueue/extension/storage/mock/storage_mock.go b/submitqueue/orchestrator/extension/storage/mock/storage_mock.go similarity index 97% rename from submitqueue/extension/storage/mock/storage_mock.go rename to submitqueue/orchestrator/extension/storage/mock/storage_mock.go index 7831d4fba..40a2fe729 100644 --- a/submitqueue/extension/storage/mock/storage_mock.go +++ b/submitqueue/orchestrator/extension/storage/mock/storage_mock.go @@ -13,6 +13,7 @@ import ( reflect "reflect" storage "github.com/uber/submitqueue/submitqueue/extension/storage" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" gomock "go.uber.org/mock/gomock" ) @@ -41,10 +42,10 @@ func (m *MockFactory) EXPECT() *MockFactoryMockRecorder { } // For mocks base method. -func (m *MockFactory) For(config storage.Config) (storage.Storage, error) { +func (m *MockFactory) For(config orchstorage.Config) (orchstorage.Storage, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "For", config) - ret0, _ := ret[0].(storage.Storage) + ret0, _ := ret[0].(orchstorage.Storage) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/submitqueue/extension/storage/mysql/BUILD.bazel b/submitqueue/orchestrator/extension/storage/mysql/BUILD.bazel similarity index 91% rename from submitqueue/extension/storage/mysql/BUILD.bazel rename to submitqueue/orchestrator/extension/storage/mysql/BUILD.bazel index dd06b0ee6..1ebf683c4 100644 --- a/submitqueue/extension/storage/mysql/BUILD.bazel +++ b/submitqueue/orchestrator/extension/storage/mysql/BUILD.bazel @@ -14,12 +14,13 @@ go_library( "speculation_path_set_store.go", "storage.go", ], - importpath = "github.com/uber/submitqueue/submitqueue/extension/storage/mysql", + importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mysql", visibility = ["//visibility:public"], deps = [ "//platform/metrics:go_default_library", "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "@com_github_go_sql_driver_mysql//:go_default_library", "@com_github_uber_go_tally//:go_default_library", ], diff --git a/submitqueue/extension/storage/mysql/batch_dependent_store.go b/submitqueue/orchestrator/extension/storage/mysql/batch_dependent_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/batch_dependent_store.go rename to submitqueue/orchestrator/extension/storage/mysql/batch_dependent_store.go diff --git a/submitqueue/extension/storage/mysql/batch_dependent_store_test.go b/submitqueue/orchestrator/extension/storage/mysql/batch_dependent_store_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/batch_dependent_store_test.go rename to submitqueue/orchestrator/extension/storage/mysql/batch_dependent_store_test.go diff --git a/submitqueue/extension/storage/mysql/batch_store.go b/submitqueue/orchestrator/extension/storage/mysql/batch_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/batch_store.go rename to submitqueue/orchestrator/extension/storage/mysql/batch_store.go diff --git a/submitqueue/extension/storage/mysql/batch_store_test.go b/submitqueue/orchestrator/extension/storage/mysql/batch_store_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/batch_store_test.go rename to submitqueue/orchestrator/extension/storage/mysql/batch_store_test.go diff --git a/submitqueue/extension/storage/mysql/build_store.go b/submitqueue/orchestrator/extension/storage/mysql/build_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/build_store.go rename to submitqueue/orchestrator/extension/storage/mysql/build_store.go diff --git a/submitqueue/extension/storage/mysql/build_store_test.go b/submitqueue/orchestrator/extension/storage/mysql/build_store_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/build_store_test.go rename to submitqueue/orchestrator/extension/storage/mysql/build_store_test.go diff --git a/submitqueue/extension/storage/mysql/change_store.go b/submitqueue/orchestrator/extension/storage/mysql/change_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/change_store.go rename to submitqueue/orchestrator/extension/storage/mysql/change_store.go diff --git a/submitqueue/extension/storage/mysql/change_store_test.go b/submitqueue/orchestrator/extension/storage/mysql/change_store_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/change_store_test.go rename to submitqueue/orchestrator/extension/storage/mysql/change_store_test.go diff --git a/submitqueue/extension/storage/mysql/path_build_store.go b/submitqueue/orchestrator/extension/storage/mysql/path_build_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/path_build_store.go rename to submitqueue/orchestrator/extension/storage/mysql/path_build_store.go diff --git a/submitqueue/extension/storage/mysql/queue_batch_state_store.go b/submitqueue/orchestrator/extension/storage/mysql/queue_batch_state_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/queue_batch_state_store.go rename to submitqueue/orchestrator/extension/storage/mysql/queue_batch_state_store.go diff --git a/submitqueue/extension/storage/mysql/queue_batch_state_store_test.go b/submitqueue/orchestrator/extension/storage/mysql/queue_batch_state_store_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/queue_batch_state_store_test.go rename to submitqueue/orchestrator/extension/storage/mysql/queue_batch_state_store_test.go diff --git a/submitqueue/extension/storage/mysql/request_batch_store.go b/submitqueue/orchestrator/extension/storage/mysql/request_batch_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/request_batch_store.go rename to submitqueue/orchestrator/extension/storage/mysql/request_batch_store.go diff --git a/submitqueue/extension/storage/mysql/request_batch_store_test.go b/submitqueue/orchestrator/extension/storage/mysql/request_batch_store_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/request_batch_store_test.go rename to submitqueue/orchestrator/extension/storage/mysql/request_batch_store_test.go diff --git a/submitqueue/extension/storage/mysql/request_store.go b/submitqueue/orchestrator/extension/storage/mysql/request_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/request_store.go rename to submitqueue/orchestrator/extension/storage/mysql/request_store.go diff --git a/submitqueue/extension/storage/mysql/request_store_test.go b/submitqueue/orchestrator/extension/storage/mysql/request_store_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/request_store_test.go rename to submitqueue/orchestrator/extension/storage/mysql/request_store_test.go diff --git a/submitqueue/extension/storage/mysql/schema/BUILD.bazel b/submitqueue/orchestrator/extension/storage/mysql/schema/BUILD.bazel similarity index 100% rename from submitqueue/extension/storage/mysql/schema/BUILD.bazel rename to submitqueue/orchestrator/extension/storage/mysql/schema/BUILD.bazel diff --git a/submitqueue/extension/storage/mysql/schema/README.md b/submitqueue/orchestrator/extension/storage/mysql/schema/README.md similarity index 100% rename from submitqueue/extension/storage/mysql/schema/README.md rename to submitqueue/orchestrator/extension/storage/mysql/schema/README.md diff --git a/submitqueue/extension/storage/mysql/schema/batch.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/batch.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/batch.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/batch.sql diff --git a/submitqueue/extension/storage/mysql/schema/batch_dependent.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/batch_dependent.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/batch_dependent.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/batch_dependent.sql diff --git a/submitqueue/extension/storage/mysql/schema/build.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/build.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/build.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/build.sql diff --git a/submitqueue/extension/storage/mysql/schema/change.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/change.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/change.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/change.sql diff --git a/submitqueue/extension/storage/mysql/schema/path_build.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/path_build.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/path_build.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/path_build.sql diff --git a/submitqueue/extension/storage/mysql/schema/queue_batch_state.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/queue_batch_state.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/queue_batch_state.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/queue_batch_state.sql diff --git a/submitqueue/extension/storage/mysql/schema/request.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/request.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/request.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/request.sql diff --git a/submitqueue/extension/storage/mysql/schema/request_batch.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/request_batch.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/request_batch.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/request_batch.sql diff --git a/submitqueue/extension/storage/mysql/schema/speculation_path_set.sql b/submitqueue/orchestrator/extension/storage/mysql/schema/speculation_path_set.sql similarity index 100% rename from submitqueue/extension/storage/mysql/schema/speculation_path_set.sql rename to submitqueue/orchestrator/extension/storage/mysql/schema/speculation_path_set.sql diff --git a/submitqueue/extension/storage/mysql/speculation_path_set_store.go b/submitqueue/orchestrator/extension/storage/mysql/speculation_path_set_store.go similarity index 100% rename from submitqueue/extension/storage/mysql/speculation_path_set_store.go rename to submitqueue/orchestrator/extension/storage/mysql/speculation_path_set_store.go diff --git a/submitqueue/extension/storage/mysql/speculation_path_set_store_test.go b/submitqueue/orchestrator/extension/storage/mysql/speculation_path_set_store_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/speculation_path_set_store_test.go rename to submitqueue/orchestrator/extension/storage/mysql/speculation_path_set_store_test.go diff --git a/submitqueue/extension/storage/mysql/storage.go b/submitqueue/orchestrator/extension/storage/mysql/storage.go similarity index 75% rename from submitqueue/extension/storage/mysql/storage.go rename to submitqueue/orchestrator/extension/storage/mysql/storage.go index 8c59717d2..1309ee86d 100644 --- a/submitqueue/extension/storage/mysql/storage.go +++ b/submitqueue/orchestrator/extension/storage/mysql/storage.go @@ -21,7 +21,8 @@ import ( _ "github.com/go-sql-driver/mysql" "github.com/uber-go/tally" - "github.com/uber/submitqueue/submitqueue/extension/storage" + basestorage "github.com/uber/submitqueue/submitqueue/extension/storage" + storage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" ) // mysqlErrDuplicateEntry is MySQL error code 1062 ("Duplicate entry"), returned on a unique or primary key violation. @@ -69,61 +70,61 @@ func (s *Storage) Close() error { // boundStorage is the queue-scoped store aggregate returned by For. type boundStorage struct { - requestStore storage.RequestStore - requestBatchStore storage.RequestBatchStore - changeStore storage.ChangeStore - batchStore storage.BatchStore - batchDependentStore storage.BatchDependentStore - queueBatchStateStore storage.QueueBatchStateStore - buildStore storage.BuildStore - speculationPathSetStore storage.SpeculationPathSetStore - pathBuildStore storage.PathBuildStore + requestStore basestorage.RequestStore + requestBatchStore basestorage.RequestBatchStore + changeStore basestorage.ChangeStore + batchStore basestorage.BatchStore + batchDependentStore basestorage.BatchDependentStore + queueBatchStateStore basestorage.QueueBatchStateStore + buildStore basestorage.BuildStore + speculationPathSetStore basestorage.SpeculationPathSetStore + pathBuildStore basestorage.PathBuildStore } // Verify boundStorage implements the queue-scoped aggregate at compile time. var _ storage.Storage = (*boundStorage)(nil) // GetRequestStore returns the bound MySQL-backed RequestStore. -func (f *boundStorage) GetRequestStore() storage.RequestStore { +func (f *boundStorage) GetRequestStore() basestorage.RequestStore { return f.requestStore } // GetRequestBatchStore returns the bound MySQL-backed RequestBatchStore. -func (f *boundStorage) GetRequestBatchStore() storage.RequestBatchStore { +func (f *boundStorage) GetRequestBatchStore() basestorage.RequestBatchStore { return f.requestBatchStore } // GetChangeStore returns the bound MySQL-backed ChangeStore. -func (f *boundStorage) GetChangeStore() storage.ChangeStore { +func (f *boundStorage) GetChangeStore() basestorage.ChangeStore { return f.changeStore } // GetBatchStore returns the bound MySQL-backed BatchStore. -func (f *boundStorage) GetBatchStore() storage.BatchStore { +func (f *boundStorage) GetBatchStore() basestorage.BatchStore { return f.batchStore } // GetBatchDependentStore returns the bound MySQL-backed BatchDependentStore. -func (f *boundStorage) GetBatchDependentStore() storage.BatchDependentStore { +func (f *boundStorage) GetBatchDependentStore() basestorage.BatchDependentStore { return f.batchDependentStore } // GetQueueBatchStateStore returns the bound MySQL-backed QueueBatchStateStore. -func (f *boundStorage) GetQueueBatchStateStore() storage.QueueBatchStateStore { +func (f *boundStorage) GetQueueBatchStateStore() basestorage.QueueBatchStateStore { return f.queueBatchStateStore } // GetBuildStore returns the bound MySQL-backed BuildStore. -func (f *boundStorage) GetBuildStore() storage.BuildStore { +func (f *boundStorage) GetBuildStore() basestorage.BuildStore { return f.buildStore } // GetSpeculationPathSetStore returns the bound MySQL-backed SpeculationPathSetStore. -func (f *boundStorage) GetSpeculationPathSetStore() storage.SpeculationPathSetStore { +func (f *boundStorage) GetSpeculationPathSetStore() basestorage.SpeculationPathSetStore { return f.speculationPathSetStore } // GetPathBuildStore returns the bound MySQL-backed PathBuildStore. -func (f *boundStorage) GetPathBuildStore() storage.PathBuildStore { +func (f *boundStorage) GetPathBuildStore() basestorage.PathBuildStore { return f.pathBuildStore } diff --git a/submitqueue/extension/storage/mysql/storage_test.go b/submitqueue/orchestrator/extension/storage/mysql/storage_test.go similarity index 100% rename from submitqueue/extension/storage/mysql/storage_test.go rename to submitqueue/orchestrator/extension/storage/mysql/storage_test.go diff --git a/submitqueue/orchestrator/extension/storage/storage.go b/submitqueue/orchestrator/extension/storage/storage.go new file mode 100644 index 000000000..5b6ebb4c0 --- /dev/null +++ b/submitqueue/orchestrator/extension/storage/storage.go @@ -0,0 +1,73 @@ +// Copyright (c) 2025 Uber Technologies, 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 storage resolves the orchestrator's queue-scoped stores: the +// pipeline working state behind validation, batching, speculation, building +// and landing. The store contracts themselves stay in +// submitqueue/extension/storage — only the aggregate is service-scoped, so +// what the orchestrator can reach is narrower than what the domain defines. +package storage + +//go:generate mockgen -source=storage.go -destination=mock/storage_mock.go -package=mock + +import ( + basestorage "github.com/uber/submitqueue/submitqueue/extension/storage" +) + +// Config identifies the queue a Storage instance is resolved for. It is the +// shared storage config: an alias rather than a new type, so a caller holding +// one can resolve either service's storage with it. +type Config = basestorage.Config + +// Factory resolves the queue-scoped Storage aggregate for a queue. Mirrors the +// extension contract: the host wiring decides which backend serves which +// queue; implementations bind the queue over their backend so a resolved +// instance can only read and write that queue's data. +type Factory interface { + // For returns the Storage aggregate bound to the queue named in config. + For(config Config) (Storage, error) +} + +// Storage aggregates the orchestrator's queue-scoped stores into a single +// injectable dependency. An instance is resolved per queue through Factory and +// is bound to that queue: entity arguments whose Queue field disagrees with +// the binding are rejected, and reads never surface another queue's records. +type Storage interface { + // GetRequestStore returns the RequestStore instance. + GetRequestStore() basestorage.RequestStore + + // GetRequestBatchStore returns the RequestBatchStore instance. + GetRequestBatchStore() basestorage.RequestBatchStore + + // GetChangeStore returns the ChangeStore instance. + GetChangeStore() basestorage.ChangeStore + + // GetBatchStore returns the BatchStore instance. + GetBatchStore() basestorage.BatchStore + + // GetBatchDependentStore returns the BatchDependentStore instance. + GetBatchDependentStore() basestorage.BatchDependentStore + + // GetQueueBatchStateStore returns the QueueBatchStateStore instance. + GetQueueBatchStateStore() basestorage.QueueBatchStateStore + + // GetBuildStore returns the BuildStore instance. + GetBuildStore() basestorage.BuildStore + + // GetSpeculationPathSetStore returns the SpeculationPathSetStore instance. + GetSpeculationPathSetStore() basestorage.SpeculationPathSetStore + + // GetPathBuildStore returns the PathBuildStore instance. + GetPathBuildStore() basestorage.PathBuildStore +} diff --git a/submitqueue/orchestrator/pipeline.go b/submitqueue/orchestrator/pipeline.go index 23d6630f7..bcdadacb9 100644 --- a/submitqueue/orchestrator/pipeline.go +++ b/submitqueue/orchestrator/pipeline.go @@ -33,7 +33,6 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/changeprovider" "github.com/uber/submitqueue/submitqueue/extension/conflict" "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator" - "github.com/uber/submitqueue/submitqueue/extension/storage" "github.com/uber/submitqueue/submitqueue/extension/validator" "github.com/uber/submitqueue/submitqueue/orchestrator/controller" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/batch" @@ -49,6 +48,7 @@ import ( "github.com/uber/submitqueue/submitqueue/orchestrator/controller/speculate" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/start" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/validate" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" "go.uber.org/zap" ) @@ -64,7 +64,7 @@ type Deps struct { Scope tally.Scope // Storage resolves the queue-scoped store aggregate per queue. - Storage storage.Factory + Storage orchstorage.Factory // Counter resolves the queue-scoped batch counter per queue. Counter counter.Factory diff --git a/test/e2e/submitqueue/BUILD.bazel b/test/e2e/submitqueue/BUILD.bazel index b7d436f60..6ff69928c 100644 --- a/test/e2e/submitqueue/BUILD.bazel +++ b/test/e2e/submitqueue/BUILD.bazel @@ -51,7 +51,7 @@ go_test( "//submitqueue/core/topickey:go_default_library", "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", - "//submitqueue/extension/storage/mysql:go_default_library", + "//submitqueue/orchestrator/extension/storage/mysql:go_default_library", "//test/testutil:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", diff --git a/test/e2e/submitqueue/suite_test.go b/test/e2e/submitqueue/suite_test.go index 18d855d97..98335bca3 100644 --- a/test/e2e/submitqueue/suite_test.go +++ b/test/e2e/submitqueue/suite_test.go @@ -43,7 +43,7 @@ import ( orchestratorpb "github.com/uber/submitqueue/api/submitqueue/orchestrator/protopb" consumergatefile "github.com/uber/submitqueue/platform/extension/consumergate/file" "github.com/uber/submitqueue/submitqueue/entity" - storagemysql "github.com/uber/submitqueue/submitqueue/extension/storage/mysql" + storagemysql "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mysql" "github.com/uber/submitqueue/test/testutil" "google.golang.org/grpc" "google.golang.org/grpc/codes" diff --git a/test/integration/submitqueue/extension/storage/BUILD.bazel b/test/integration/submitqueue/extension/storage/BUILD.bazel index 3c6736468..e844c7337 100644 --- a/test/integration/submitqueue/extension/storage/BUILD.bazel +++ b/test/integration/submitqueue/extension/storage/BUILD.bazel @@ -11,6 +11,7 @@ go_library( "//submitqueue/entity:go_default_library", "//submitqueue/extension/storage:go_default_library", "//submitqueue/gateway/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", "//test/testutil:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", diff --git a/test/integration/submitqueue/extension/storage/mysql/BUILD.bazel b/test/integration/submitqueue/extension/storage/mysql/BUILD.bazel index b1e7942ad..fc8b7f484 100644 --- a/test/integration/submitqueue/extension/storage/mysql/BUILD.bazel +++ b/test/integration/submitqueue/extension/storage/mysql/BUILD.bazel @@ -5,18 +5,18 @@ go_test( srcs = ["storage_test.go"], data = [ "docker-compose.yml", - "//submitqueue/extension/storage/mysql/schema", "//submitqueue/gateway/extension/storage/mysql/schema", + "//submitqueue/orchestrator/extension/storage/mysql/schema", ], tags = [ "integration", "requires-network", ], deps = [ - "//submitqueue/extension/storage:go_default_library", - "//submitqueue/extension/storage/mysql:go_default_library", "//submitqueue/gateway/extension/storage:go_default_library", "//submitqueue/gateway/extension/storage/mysql:go_default_library", + "//submitqueue/orchestrator/extension/storage:go_default_library", + "//submitqueue/orchestrator/extension/storage/mysql:go_default_library", "//test/integration/submitqueue/extension/storage:go_default_library", "//test/testutil:go_default_library", "@com_github_go_sql_driver_mysql//:go_default_library", diff --git a/test/integration/submitqueue/extension/storage/mysql/storage_test.go b/test/integration/submitqueue/extension/storage/mysql/storage_test.go index 3c8da0d9e..bbc0f07a2 100644 --- a/test/integration/submitqueue/extension/storage/mysql/storage_test.go +++ b/test/integration/submitqueue/extension/storage/mysql/storage_test.go @@ -19,14 +19,15 @@ import ( "database/sql" "testing" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/uber-go/tally" - "github.com/uber/submitqueue/submitqueue/extension/storage" - mysqlstorage "github.com/uber/submitqueue/submitqueue/extension/storage/mysql" gwstorage "github.com/uber/submitqueue/submitqueue/gateway/extension/storage" gwmysqlstorage "github.com/uber/submitqueue/submitqueue/gateway/extension/storage/mysql" + mysqlstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage/mysql" storagesuite "github.com/uber/submitqueue/test/integration/submitqueue/extension/storage" "github.com/uber/submitqueue/test/testutil" ) @@ -71,7 +72,7 @@ func (s *MySQLStorageIntegrationSuite) SetupSuite() { require.NoError(t, err, "failed to connect to MySQL") // Apply schemas programmatically from directory - testutil.ApplySchema(t, s.log, s.db, testutil.SchemaDir("submitqueue/extension/storage/mysql/schema")) + testutil.ApplySchema(t, s.log, s.db, testutil.SchemaDir("submitqueue/orchestrator/extension/storage/mysql/schema")) testutil.ApplySchema(t, s.log, s.db, testutil.SchemaDir("submitqueue/gateway/extension/storage/mysql/schema")) s.log.Logf("Schemas applied successfully") @@ -106,18 +107,18 @@ func (s *MySQLStorageIntegrationSuite) TearDownSuite() { } // mysqlFactory adapts the MySQL storage backend's queue binding to the -// storage.Factory seam for the contract suite, mirroring the host wiring. +// orchstorage.Factory seam for the contract suite, mirroring the host wiring. type mysqlFactory struct { backend *mysqlstorage.Storage } // For returns the queue-scoped store aggregate bound to the queue named in config. -func (f mysqlFactory) For(config storage.Config) (storage.Storage, error) { +func (f mysqlFactory) For(config orchstorage.Config) (orchstorage.Storage, error) { return f.backend.For(config.QueueName) } // gatewayMySQLFactory adapts the gateway's MySQL storage backend to the -// gateway storage.Factory seam for the contract suite. +// gateway orchstorage.Factory seam for the contract suite. type gatewayMySQLFactory struct { backend *gwmysqlstorage.Storage } diff --git a/test/integration/submitqueue/extension/storage/suite.go b/test/integration/submitqueue/extension/storage/suite.go index 79597c280..23caca7bd 100644 --- a/test/integration/submitqueue/extension/storage/suite.go +++ b/test/integration/submitqueue/extension/storage/suite.go @@ -20,6 +20,8 @@ import ( "sort" "testing" + orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -32,13 +34,13 @@ import ( ) // StorageContractSuite defines the contract tests for the storage extension: -// the queue-scoped aggregate resolved through storage.Factory. All storage +// the queue-scoped aggregate resolved through orchstorage.Factory. All storage // implementations must pass these tests. Implementation-specific tests should // embed this suite and call SetFactory(). type StorageContractSuite struct { suite.Suite ctx context.Context - factory storage.Factory + factory orchstorage.Factory gatewayFactory gwstorage.Factory log *testutil.TestLogger } @@ -50,7 +52,7 @@ func (s *StorageContractSuite) SetContext(ctx context.Context) { // SetFactory is called by implementation tests to provide the queue-scoped // storage factory under test. -func (s *StorageContractSuite) SetFactory(factory storage.Factory) { +func (s *StorageContractSuite) SetFactory(factory orchstorage.Factory) { s.factory = factory } @@ -70,8 +72,8 @@ func (s *StorageContractSuite) forGatewayQueue(queue string) gwstorage.Storage { // forQueue resolves the queue-scoped store aggregate for a queue, failing the // test on resolution errors. -func (s *StorageContractSuite) forQueue(queue string) storage.Storage { - store, err := s.factory.For(storage.Config{QueueName: queue}) +func (s *StorageContractSuite) forQueue(queue string) orchstorage.Storage { + store, err := s.factory.For(orchstorage.Config{QueueName: queue}) s.Require().NoError(err) return store } diff --git a/test/integration/submitqueue/orchestrator/BUILD.bazel b/test/integration/submitqueue/orchestrator/BUILD.bazel index a81f9d017..ca5158a79 100644 --- a/test/integration/submitqueue/orchestrator/BUILD.bazel +++ b/test/integration/submitqueue/orchestrator/BUILD.bazel @@ -8,7 +8,7 @@ go_test( "//platform/extension/messagequeue/mysql/schema", "//service/submitqueue/orchestrator/server:docker-compose.yml", "//service/submitqueue/orchestrator/server:docker_test_context", - "//submitqueue/extension/storage/mysql/schema", + "//submitqueue/orchestrator/extension/storage/mysql/schema", ], tags = [ "integration", diff --git a/test/integration/submitqueue/orchestrator/suite_test.go b/test/integration/submitqueue/orchestrator/suite_test.go index 2d84ecf5f..e04952747 100644 --- a/test/integration/submitqueue/orchestrator/suite_test.go +++ b/test/integration/submitqueue/orchestrator/suite_test.go @@ -83,7 +83,7 @@ func (s *OrchestratorIntegrationSuite) SetupSuite() { require.NoError(t, err, "failed to connect to queue MySQL") // Apply schemas programmatically to application database - testutil.ApplySchema(t, s.log, s.db, testutil.SchemaDir("submitqueue/extension/storage/mysql/schema")) + testutil.ApplySchema(t, s.log, s.db, testutil.SchemaDir("submitqueue/orchestrator/extension/storage/mysql/schema")) testutil.ApplySchema(t, s.log, s.db, testutil.SchemaDir("platform/extension/counter/mysql/schema")) // Apply schemas programmatically to queue database diff --git a/test/testutil/schema.go b/test/testutil/schema.go index fff9bc9bc..6bbdf4e2f 100644 --- a/test/testutil/schema.go +++ b/test/testutil/schema.go @@ -41,7 +41,7 @@ func Runfile(relativePath string) string { // SchemaDir returns the path to a schema directory. // It checks for both Bazel runfiles and direct go test paths. -// relativePath should be like "submitqueue/extension/storage/mysql/schema" or "platform/extension/messagequeue/mysql/schema" +// relativePath should be like "submitqueue/orchestrator/extension/storage/mysql/schema" or "platform/extension/messagequeue/mysql/schema" func SchemaDir(relativePath string) string { return Runfile(relativePath) } @@ -56,7 +56,7 @@ func SchemaDir(relativePath string) string { func SubmitQueueStorageSchemaDirs() []string { return []string{ "submitqueue/gateway/extension/storage/mysql/schema", - "submitqueue/extension/storage/mysql/schema", + "submitqueue/orchestrator/extension/storage/mysql/schema", } } diff --git a/tool/linter/queueshard/main.go b/tool/linter/queueshard/main.go index 0b880e3f7..ef7509548 100644 --- a/tool/linter/queueshard/main.go +++ b/tool/linter/queueshard/main.go @@ -34,16 +34,16 @@ import ( // schemaShardColumns identifies the allowed shard columns for each schema root. var schemaShardColumns = map[string]map[string]bool{ - "submitqueue/extension/storage/mysql/schema": {"queue": true, "name": true}, - "submitqueue/gateway/extension/storage/mysql/schema": {"queue": true, "name": true}, - "stovepipe/extension/storage/mysql/schema": {"queue": true, "name": true}, - "platform/extension/counter/mysql/schema": {"queue": true, "name": true}, - "platform/extension/messagequeue/mysql/schema": {"tenant": true}, + "submitqueue/orchestrator/extension/storage/mysql/schema": {"queue": true, "name": true}, + "submitqueue/gateway/extension/storage/mysql/schema": {"queue": true, "name": true}, + "stovepipe/extension/storage/mysql/schema": {"queue": true, "name": true}, + "platform/extension/counter/mysql/schema": {"queue": true, "name": true}, + "platform/extension/messagequeue/mysql/schema": {"tenant": true}, } // schemaRoots are the directories scanned for table definitions. var schemaRoots = []string{ - "submitqueue/extension/storage/mysql/schema", + "submitqueue/orchestrator/extension/storage/mysql/schema", "submitqueue/gateway/extension/storage/mysql/schema", "stovepipe/extension/storage/mysql/schema", "platform/extension/counter/mysql/schema",