[raft/memstore] Add scd memstore by the-glu · Pull Request #1542 · interuss/dss

the-glu · 2026-06-17T07:49:38Z

This PR is part of a new chain, implementing memstore.

#1525 (Generic raftstore) -> #1527 (Base memstore) -> #1529 (Aux memstore) -> #1530 (Snapshots) -> #1534 (Rid memstore) -> #1535 (Correct use of now) -> #1539 (Checkpoint) -> #1542 (Scd memstore) -> #1528 (First PR with raft using memstore)

It adds SCD memstore.

The PR is quite big, as it's the whole SCD memstore. However, individual files can be checked separately.

There were no existing tests compared to RID, except for expired operational intents / subscriptions. Those two have been copied, and the rest is new.

the-glu · 2026-07-31T07:31:22Z

+	return true
+}
+
+func clonePtr[T any](v *T) *T {


question: we should probably move common function to a utils package right? Or should every memstore kept it owns helpers ?

Agreed, it could simply be in pkg/memstore

the-glu · 2026-07-31T07:31:48Z

@mickmis That one should be ready for a review

mickmis

LGTM modulo minor comments

mickmis · 2026-08-04T13:11:17Z

+	require.NoError(t, err)
+	require.Equal(t, manager, got.Uss)
+	require.Equal(t, scdmodels.UssAvailabilityStateNormal, got.Availability)
+	require.NotEmpty(t, got.Version)


Check actual value? Hardcoding expectation here is fine IMO

I changed, but it's a bit weak (changing computation mean changing it, witch is not great)

I think it's a good thing actually? IMO it is very acceptable, desirable even, to have to change the value here if we change how we compute the OVN, because it is part of the behavior of the function we are unit testing.

Depend on what you want to test. I'm more to test "something is returned" and "it's the same thing that is returned later", not "the value is generated the way I expected because I froze time/random", that not really IMO the goal of the test. The requirement is to return a value with some characteristics, not a value we choose and enforce because of it's characteristics.

Should the version be slightly different and the function implementing change, changing all tests to conform to the new value (and probably by taking the test message to get the new one) don't add much.

(But IMO/nits ^^')

mickmis · 2026-08-04T13:17:34Z

+	return true
+}
+
+func clonePtr[T any](v *T) *T {


Agreed, it could simply be in pkg/memstore

mickmis · 2026-08-04T13:25:42Z

-func (r *repo) UpsertConstraint(_ context.Context, constraint *scdmodels.Constraint) (*scdmodels.Constraint, error) {
-	return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpsertConstraint not implemented for memstore")
+func (r *repo) UpsertConstraint(ctx context.Context, s *scdmodels.Constraint) (*scdmodels.Constraint, error) {
+	if _, err := dsssql.CellUnionToCellIdsWithValidation(s.Cells); err != nil {


Just validate with geo.ValidateCell then?

It's a small helper on the loop + convert, is it worth it to change it back?

nit: I'd factor away the loop in utils.go and use it here. But is it worth it? Meh up to you.

mickmis · 2026-08-05T12:24:18Z

+	require.NoError(t, err)
+	require.Equal(t, manager, got.Uss)
+	require.Equal(t, scdmodels.UssAvailabilityStateNormal, got.Availability)
+	require.NotEmpty(t, got.Version)


I think it's a good thing actually? IMO it is very acceptable, desirable even, to have to change the value here if we change how we compute the OVN, because it is part of the behavior of the function we are unit testing.

mickmis · 2026-08-05T12:25:00Z

-func (r *repo) UpsertConstraint(_ context.Context, constraint *scdmodels.Constraint) (*scdmodels.Constraint, error) {
-	return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpsertConstraint not implemented for memstore")
+func (r *repo) UpsertConstraint(ctx context.Context, s *scdmodels.Constraint) (*scdmodels.Constraint, error) {
+	if _, err := dsssql.CellUnionToCellIdsWithValidation(s.Cells); err != nil {


nit: I'd factor away the loop in utils.go and use it here. But is it worth it? Meh up to you.

MariemBaccari reviewed Jun 17, 2026

View reviewed changes

Comment thread pkg/scd/store/memstore/availability.go Outdated

the-glu force-pushed the memstore_scd branch from 255a3a3 to c3f668d Compare June 17, 2026 08:13

This was referenced Jun 17, 2026

[memstore/rid] Don't allow nil value for start/end time #1543

Open

[memstore/scd] Remove legacy nil cases #1544

Open

[raft] Add prometheus metrics and dashboard #1545

Open

barroco added the dss-raft Relating to the application-layer consensus implemenation based on raft label Jun 17, 2026

the-glu force-pushed the memstore_scd branch 2 times, most recently from 04a7568 to ee03a27 Compare June 18, 2026 08:47

This was referenced Jun 18, 2026

[raft/aux] Add aux Raftstore #1528

Open

[raft/rid] Add rid raftstore ISAs #1540

Closed

[raft/rid] Add rid raftstore subscriptions #1548

Closed

[raft/scd] Add scd Raftstore opintents #1553

Closed

the-glu force-pushed the memstore_scd branch from ee03a27 to 0457745 Compare June 19, 2026 13:28

This was referenced Jun 19, 2026

[raft/scd] Add scd Raftstore availability #1556

Closed

[raft/scd] Add scd Raftstore constraints #1555

Closed

[raft/scd] Add scd Raftstore subscriptions #1554

Closed

the-glu force-pushed the memstore_scd branch 2 times, most recently from 7ea03cd to e55c786 Compare June 29, 2026 07:05

the-glu force-pushed the memstore_scd branch 3 times, most recently from 9a30121 to 00ce64a Compare July 1, 2026 09:24

the-glu force-pushed the memstore_scd branch from 00ce64a to c9ef3a4 Compare July 13, 2026 08:58

github-project-automation Bot added this to InterUSS Backlog Jul 22, 2026

the-glu force-pushed the memstore_scd branch from c9ef3a4 to d8f744c Compare July 31, 2026 06:54

the-glu force-pushed the memstore_scd branch 2 times, most recently from b703298 to 8ad6994 Compare July 31, 2026 07:30

the-glu commented Jul 31, 2026

View reviewed changes

mickmis approved these changes Aug 4, 2026

View reviewed changes

the-glu force-pushed the memstore_scd branch 2 times, most recently from 5a425af to e607151 Compare August 4, 2026 16:34

[raft/memstore] Add scd memstore

8bf2039

the-glu force-pushed the memstore_scd branch from e607151 to 8bf2039 Compare August 4, 2026 16:39

mickmis approved these changes Aug 5, 2026

View reviewed changes

mickmis merged commit a7ee1f3 into interuss:master Aug 5, 2026
12 checks passed

github-project-automation Bot moved this to Done in InterUSS Backlog Aug 5, 2026

mickmis deleted the memstore_scd branch August 5, 2026 12:25

Uh oh!

Conversation

the-glu commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the-glu commented Jul 31, 2026

Uh oh!

mickmis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

the-glu commented Jun 17, 2026 •

edited

Loading