From 8321be0b0af59580a2a1cd9569e2507da093c9dc Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 22 May 2026 09:23:39 +0200 Subject: [PATCH] [raft] change transact signature --- cmds/db-manager/cleanup/evict.go | 5 +++-- pkg/raftstore/store.go | 5 +++-- pkg/rid/application/application_test.go | 5 +++-- pkg/rid/application/isa.go | 7 ++++--- pkg/rid/application/subscription.go | 7 ++++--- pkg/rid/store/sqlstore/store_test.go | 9 +++++---- pkg/scd/constraints_handler.go | 9 +++++---- pkg/scd/operational_intents_handler.go | 9 +++++---- pkg/scd/subscriptions_handler.go | 9 +++++---- pkg/scd/uss_availability_handler.go | 5 +++-- pkg/sqlstore/store.go | 6 +++--- pkg/store/store.go | 14 +++++++++++--- 12 files changed, 54 insertions(+), 36 deletions(-) diff --git a/cmds/db-manager/cleanup/evict.go b/cmds/db-manager/cleanup/evict.go index 572b90f5b..f8c144db9 100644 --- a/cmds/db-manager/cleanup/evict.go +++ b/cmds/db-manager/cleanup/evict.go @@ -14,6 +14,7 @@ import ( scdmodels "github.com/interuss/dss/pkg/scd/models" scdrepos "github.com/interuss/dss/pkg/scd/repos" scds "github.com/interuss/dss/pkg/scd/store" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -100,7 +101,7 @@ func evict(cmd *cobra.Command, _ []string) error { } return nil } - if err = scdStore.Transact(ctx, scdAction); err != nil { + if _, err = scdStore.Transact(ctx, store.Request{}, scdAction); err != nil { return fmt.Errorf("failed to execute SCD transaction: %w", err) } @@ -145,7 +146,7 @@ func evict(cmd *cobra.Command, _ []string) error { return nil } - if err = ridStore.Transact(ctx, ridAction); err != nil { + if _, err = ridStore.Transact(ctx, store.Request{}, ridAction); err != nil { return fmt.Errorf("failed to execute RID transaction: %w", err) } diff --git a/pkg/raftstore/store.go b/pkg/raftstore/store.go index ee2b40b51..7f0b8b2af 100644 --- a/pkg/raftstore/store.go +++ b/pkg/raftstore/store.go @@ -5,6 +5,7 @@ import ( "github.com/interuss/dss/pkg/raftstore/consensus" raftparams "github.com/interuss/dss/pkg/raftstore/params" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "go.uber.org/zap" ) @@ -29,9 +30,9 @@ func Init[R any](ctx context.Context, logger *zap.Logger, params raftparams.Conn } // Transact proposes the entry to Raft and blocks until it is committed and applied. -func (s *Store[R]) Transact(ctx context.Context, f func(context.Context, R) error) error { +func (s *Store[R]) Transact(ctx context.Context, _ store.Request, f func(context.Context, R) error) (any, error) { // TODO: implement - return nil + return nil, nil } // Interact returns a repository that can be used to query the store without proposing a Raft entry. diff --git a/pkg/rid/application/application_test.go b/pkg/rid/application/application_test.go index 0cc3fd885..f4e099495 100644 --- a/pkg/rid/application/application_test.go +++ b/pkg/rid/application/application_test.go @@ -13,6 +13,7 @@ import ( dssql "github.com/interuss/dss/pkg/sql" "github.com/interuss/dss/pkg/sqlstore" "github.com/interuss/dss/pkg/sqlstore/params" + dssstore "github.com/interuss/dss/pkg/store" "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" @@ -35,8 +36,8 @@ func (s *mockRepo) Interact(ctx context.Context) (repos.Repository, error) { return s, nil } -func (s *mockRepo) Transact(ctx context.Context, f func(ctx context.Context, repo repos.Repository) error) error { - return f(ctx, s) +func (s *mockRepo) Transact(ctx context.Context, _ dssstore.Request, f func(ctx context.Context, repo repos.Repository) error) (any, error) { + return nil, f(ctx, s) } func (s *mockRepo) Close() error { diff --git a/pkg/rid/application/isa.go b/pkg/rid/application/isa.go index ea402af60..8978e720b 100644 --- a/pkg/rid/application/isa.go +++ b/pkg/rid/application/isa.go @@ -10,6 +10,7 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" "github.com/interuss/dss/pkg/rid/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" ) @@ -63,7 +64,7 @@ func (a *app) DeleteISA(ctx context.Context, id dssmodels.ID, owner dssmodels.Ow subs []*ridmodels.Subscription ) // The following will automatically retry TXN retry errors. - err := a.store.Transact(ctx, func(ctx context.Context, repo repos.Repository) error { + _, err := a.store.Transact(ctx, store.Request{}, func(ctx context.Context, repo repos.Repository) error { old, err := repo.GetISA(ctx, id, true) switch { case err != nil: @@ -104,7 +105,7 @@ func (a *app) InsertISA(ctx context.Context, isa *ridmodels.IdentificationServic subs []*ridmodels.Subscription ) // The following will automatically retry TXN retry errors. - err := a.store.Transact(ctx, func(ctx context.Context, repo repos.Repository) error { + _, err := a.store.Transact(ctx, store.Request{}, func(ctx context.Context, repo repos.Repository) error { // ensure it doesn't exist yet old, err := repo.GetISA(ctx, isa.ID, false) if err != nil { @@ -138,7 +139,7 @@ func (a *app) UpdateISA(ctx context.Context, isa *ridmodels.IdentificationServic subs []*ridmodels.Subscription ) // The following will automatically retry TXN retry errors. - err := a.store.Transact(ctx, func(ctx context.Context, repo repos.Repository) error { + _, err := a.store.Transact(ctx, store.Request{}, func(ctx context.Context, repo repos.Repository) error { var err error old, err := repo.GetISA(ctx, isa.ID, true) diff --git a/pkg/rid/application/subscription.go b/pkg/rid/application/subscription.go index 33ba2af36..a85ba25ca 100644 --- a/pkg/rid/application/subscription.go +++ b/pkg/rid/application/subscription.go @@ -8,6 +8,7 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" "github.com/interuss/dss/pkg/rid/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "go.uber.org/zap" ) @@ -60,7 +61,7 @@ func (a *app) InsertSubscription(ctx context.Context, s *ridmodels.Subscription) return nil, stacktrace.Propagate(err, "Unable to adjust time range") } var sub *ridmodels.Subscription - err := a.store.Transact(ctx, func(ctx context.Context, repo repos.Repository) error { + _, err := a.store.Transact(ctx, store.Request{}, func(ctx context.Context, repo repos.Repository) error { // ensure it doesn't exist yet old, err := repo.GetSubscription(ctx, s.ID) @@ -98,7 +99,7 @@ func (a *app) InsertSubscription(ctx context.Context, s *ridmodels.Subscription) func (a *app) UpdateSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { var sub *ridmodels.Subscription - err := a.store.Transact(ctx, func(ctx context.Context, repo repos.Repository) error { + _, err := a.store.Transact(ctx, store.Request{}, func(ctx context.Context, repo repos.Repository) error { old, err := repo.GetSubscription(ctx, s.ID) switch { case err != nil: @@ -145,7 +146,7 @@ func (a *app) UpdateSubscription(ctx context.Context, s *ridmodels.Subscription) // DeleteSubscription deletes the Subscription identified by "id" and owned by "owner". func (a *app) DeleteSubscription(ctx context.Context, id dssmodels.ID, owner dssmodels.Owner, version *dssmodels.Version) (*ridmodels.Subscription, error) { var ret *ridmodels.Subscription - err := a.store.Transact(ctx, func(ctx context.Context, repo repos.Repository) error { + _, err := a.store.Transact(ctx, store.Request{}, func(ctx context.Context, repo repos.Repository) error { var err error old, err := repo.GetSubscription(ctx, id) switch { diff --git a/pkg/rid/store/sqlstore/store_test.go b/pkg/rid/store/sqlstore/store_test.go index 275a1a332..bd19dfd29 100644 --- a/pkg/rid/store/sqlstore/store_test.go +++ b/pkg/rid/store/sqlstore/store_test.go @@ -13,6 +13,7 @@ import ( "github.com/interuss/dss/pkg/rid/repos" "github.com/interuss/dss/pkg/sqlstore" "github.com/interuss/dss/pkg/sqlstore/params" + dssstore "github.com/interuss/dss/pkg/store" "github.com/jackc/pgx/v5/pgconn" "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" @@ -97,7 +98,7 @@ func TestTxnRetrier(t *testing.T) { require.NotNil(t, store) defer tearDownStore() - err := store.Transact(ctx, func(ctx context.Context, repo repos.Repository) error { + _, err := store.Transact(ctx, dssstore.Request{}, func(ctx context.Context, repo repos.Repository) error { // can query within this isa, err := repo.InsertISA(ctx, serviceArea) require.NotNil(t, isa) @@ -118,7 +119,7 @@ func TestTxnRetrier(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, 20*time.Millisecond) defer cancel() count := 0 - err = store.Transact(ctx, func(ctx context.Context, repo repos.Repository) error { + _, err = store.Transact(ctx, dssstore.Request{}, func(ctx context.Context, repo repos.Repository) error { // can query within this count++ // Postgre retryable error @@ -141,13 +142,13 @@ func TestTransactor(t *testing.T) { subscription2 := subscriptionsPool[1].input txnCount := 0 - err := store.Transact(ctx, func(ctx context.Context, s1 repos.Repository) error { + _, err := store.Transact(ctx, dssstore.Request{}, func(ctx context.Context, s1 repos.Repository) error { // We should get to this retry, then return nothing. if txnCount > 0 { return errors.New("already failed") } txnCount++ - err := store.Transact(ctx, func(ctx context.Context, s2 repos.Repository) error { + _, err := store.Transact(ctx, dssstore.Request{}, func(ctx context.Context, s2 repos.Repository) error { subs, err := s1.SearchSubscriptions(ctx, subscription1.Cells) require.NoError(t, err) require.Len(t, subs, 0) diff --git a/pkg/scd/constraints_handler.go b/pkg/scd/constraints_handler.go index 0688176c7..fd4953c96 100644 --- a/pkg/scd/constraints_handler.go +++ b/pkg/scd/constraints_handler.go @@ -11,6 +11,7 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/dss/pkg/scd/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/jackc/pgx/v5" ) @@ -88,7 +89,7 @@ func (a *Server) DeleteConstraintReference(ctx context.Context, req *restapi.Del return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { err = stacktrace.Propagate(err, "Could not delete constraint") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -147,7 +148,7 @@ func (a *Server) GetConstraintReference(ctx context.Context, req *restapi.GetCon return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { err = stacktrace.Propagate(err, "Could not get constraint") if stacktrace.GetCode(err) == dsserr.NotFound { @@ -306,7 +307,7 @@ func (a *Server) PutConstraintReference(ctx context.Context, manager string, ent return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { return nil, err // No need to Propagate this error as this is not a useful stacktrace line } @@ -434,7 +435,7 @@ func (a *Server) QueryConstraintReferences(ctx context.Context, req *restapi.Que return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { return restapi.QueryConstraintReferencesResponseSet{Response500: &api.InternalServerErrorBody{ ErrorMessage: *dsserr.Handle(ctx, stacktrace.Propagate(err, "Got an unexpected error"))}} diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index f611797d0..9b58b4192 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -13,6 +13,7 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/dss/pkg/scd/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" ) @@ -165,7 +166,7 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { err = stacktrace.Propagate(err, "Could not delete operational intent") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -221,7 +222,7 @@ func (a *Server) GetOperationalIntentReference(ctx context.Context, req *restapi return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { err = stacktrace.Propagate(err, "Could not get operational intent") if stacktrace.GetCode(err) == dsserr.NotFound { @@ -288,7 +289,7 @@ func (a *Server) QueryOperationalIntentReferences(ctx context.Context, req *rest return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { err = stacktrace.Propagate(err, "Could not query operational intent") if stacktrace.GetCode(err) == dsserr.BadRequest { @@ -934,7 +935,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, now time. return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { return nil, responseConflict, err // No need to Propagate this error as this is not a useful stacktrace line } diff --git a/pkg/scd/subscriptions_handler.go b/pkg/scd/subscriptions_handler.go index 98ab9fed2..60a83c498 100644 --- a/pkg/scd/subscriptions_handler.go +++ b/pkg/scd/subscriptions_handler.go @@ -12,6 +12,7 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/dss/pkg/scd/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/jonboulle/clockwork" ) @@ -276,7 +277,7 @@ func (a *Server) PutSubscription(ctx context.Context, manager string, subscripti return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { return nil, err // No need to Propagate this error as this is not a useful stacktrace line } @@ -340,7 +341,7 @@ func (a *Server) GetSubscription(ctx context.Context, req *restapi.GetSubscripti return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { err = stacktrace.Propagate(err, "Could not get subscription") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -424,7 +425,7 @@ func (a *Server) QuerySubscriptions(ctx context.Context, req *restapi.QuerySubsc return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -517,7 +518,7 @@ func (a *Server) DeleteSubscription(ctx context.Context, req *restapi.DeleteSubs return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { err = stacktrace.Propagate(err, "Could not delete subscription") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} diff --git a/pkg/scd/uss_availability_handler.go b/pkg/scd/uss_availability_handler.go index 517a9859a..ee6bd48ee 100644 --- a/pkg/scd/uss_availability_handler.go +++ b/pkg/scd/uss_availability_handler.go @@ -10,6 +10,7 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/dss/pkg/scd/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/jackc/pgx/v5" ) @@ -51,7 +52,7 @@ func (a *Server) GetUssAvailability(ctx context.Context, req *restapi.GetUssAvai return nil } - err := a.Store.Transact(ctx, action) + _, err := a.Store.Transact(ctx, store.Request{}, action) if err != nil { // In case of older DB versions where availability table doesn't exist if strings.Contains(err.Error(), "does not exist") { @@ -121,7 +122,7 @@ func (a *Server) SetUssAvailability(ctx context.Context, req *restapi.SetUssAvai } return nil } - err = a.Store.Transact(ctx, action) + _, err = a.Store.Transact(ctx, store.Request{}, action) if err != nil { // In case of older DB versions where availability table doesn't exist if strings.Contains(err.Error(), "does not exist") { diff --git a/pkg/sqlstore/store.go b/pkg/sqlstore/store.go index 92437c190..a1e4ddac5 100644 --- a/pkg/sqlstore/store.go +++ b/pkg/sqlstore/store.go @@ -176,13 +176,13 @@ func (s *Store[R]) Interact(_ context.Context) (R, error) { return s.newRepo(s.Pool), nil } -func (s *Store[R]) Transact(ctx context.Context, f func(context.Context, R) error) error { +func (s *Store[R]) Transact(ctx context.Context, _ store.Request, f func(context.Context, R) error) (any, error) { if s.Version.Type == Yugabyte { - return s.transactYugabyte(ctx, f) + return nil, s.transactYugabyte(ctx, f) } ctx = crdb.WithMaxRetries(ctx, s.maxRetries) - return crdbpgx.ExecuteTx(ctx, s.Pool, pgx.TxOptions{IsoLevel: pgx.Serializable}, func(tx pgx.Tx) error { + return nil, crdbpgx.ExecuteTx(ctx, s.Pool, pgx.TxOptions{IsoLevel: pgx.Serializable}, func(tx pgx.Tx) error { return f(ctx, s.newRepo(tx)) }) } diff --git a/pkg/store/store.go b/pkg/store/store.go index e5e95c4b0..92bb3c4fc 100644 --- a/pkg/store/store.go +++ b/pkg/store/store.go @@ -7,6 +7,12 @@ import ( "github.com/interuss/stacktrace" ) +// Request carries the information needed by the Raftstore to handle a request. +type Request struct { + RequestType string + Payload any +} + // store.Store is the generic means to access and interact with any type of data backing the DSS // may ever use, by obtaining a means to perform R-specific (repo type) operations. type Store[R any] interface { @@ -14,9 +20,11 @@ type Store[R any] interface { // Obtain a Repo (repo type R) that doesn't need transactional guarantees (for instance, // read-only). Interact(context.Context) (R, error) - // Attempt to apply the operations in f to the R Repo it is supplied. All operations performed - // on the R Repo by f will be applied or rejected atomically. - Transact(ctx context.Context, f func(context.Context, R) error) error + // Transact atomically applies or rejects a request. + // f specifies the operations to be performed on the repo. + // request specifies the type and payload of request. + // any is the request result. + Transact(ctx context.Context, request Request, f func(context.Context, R) error) (any, error) } const (