Skip to content

Commit ea335ba

Browse files
behinddwallsgithub-actions[bot]
authored andcommitted
add change store extension for per-URI request claims
Introduce a dedicated extension that tracks (URI, request_id) claims with mutable metadata, scoped to queue. Foundation for cross-request URI overlap detection; no consumers in this commit. - entity/change_record.go: immutable identity (URI, RequestID), mutable metadata - extension/changestore/: ChangeStore interface, mysql impl, mock, schema - INSERT IGNORE on writes for retry idempotency on (uri, request_id) - FindOverlapping is single-table; callers do their own liveness check - integration test under test/integration/extension/changestore/mysql/ - e2e suite applies the change schema (harmless no-op until consumers land)
1 parent 7123378 commit ea335ba

16 files changed

Lines changed: 599 additions & 1 deletion

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ local-stop: ## Stop all services (keep data)
245245

246246
mocks: ## Generate mock files using mockgen
247247
@echo "Generating mocks..."
248-
@$(BAZEL) run @rules_go//go -- generate ./extension/storage/... ./extension/counter/... ./extension/queue/... ./extension/mergechecker/... ./extension/scorer/... ./core/consumer/...
248+
@$(BAZEL) run @rules_go//go -- generate ./extension/storage/... ./extension/changestore/... ./extension/counter/... ./extension/queue/... ./extension/mergechecker/... ./extension/scorer/... ./core/consumer/...
249249
@echo "Mocks generated successfully!"
250250

251251
proto: ## Generate protobuf files from .proto definitions

entity/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ go_library(
77
"batch_dependent.go",
88
"build.go",
99
"change_provider.go",
10+
"change_record.go",
1011
"land_request.go",
1112
"queue_config.go",
1213
"request.go",

entity/change_record.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package entity
16+
17+
// ChangeRecord represents a single URI's claim by a request, persisted in the change store.
18+
// The (URI, RequestID) pair is the identity and is immutable; Metadata may be updated over
19+
// time as additional information about the change (e.g., PR title, author, mergeability)
20+
// becomes available.
21+
type ChangeRecord struct {
22+
// URI identifies the change (RFC 3986). Same scheme/format as entity.Change.URIs.
23+
// Example: "github://uber/submitqueue/pull/123/abc123def".
24+
URI string `json:"uri"`
25+
26+
// RequestID is the owning land request that claimed this URI.
27+
// Format matches entity.Request.ID: "<queue>/<counter_value>".
28+
RequestID string `json:"request_id"`
29+
30+
// Queue is the queue scope for the owning request. Denormalized from the request
31+
// to allow queue-scoped duplicate checks without a join.
32+
Queue string `json:"queue"`
33+
34+
// Metadata is a JSON-encoded blob of provider-specific information about the change
35+
// (e.g., PR title, author, mergeable state). Empty when the record is first written;
36+
// populated and updated by downstream enrichment.
37+
Metadata string `json:"metadata,omitempty"`
38+
39+
// CreatedAt is the Unix milliseconds timestamp when this record was first created.
40+
CreatedAt int64 `json:"created_at"`
41+
42+
// UpdatedAt is the Unix milliseconds timestamp when this record's Metadata was last updated.
43+
// Equal to CreatedAt when the record has never been updated.
44+
UpdatedAt int64 `json:"updated_at"`
45+
}

extension/changestore/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
load("@rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "changestore",
5+
srcs = ["change_store.go"],
6+
importpath = "github.com/uber/submitqueue/extension/changestore",
7+
visibility = ["//visibility:public"],
8+
deps = ["//entity"],
9+
)

extension/changestore/README.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# ChangeStore
2+
3+
Vendor-agnostic interface for tracking per-URI claims by in-flight land requests.
4+
5+
Each record asserts that a specific URI (e.g., a GitHub PR) was claimed by a specific request, scoped to a queue. The store is consulted by the orchestrator's `start` controller to detect duplicate requests — submissions whose URIs overlap with another in-flight request's URIs in the same queue.
6+
7+
## Semantics
8+
9+
- **Identity is immutable.** A record is keyed by `(URI, RequestID)`; once written, that pair is never mutated.
10+
- **Metadata is mutable.** The `Metadata` field is intended for provider-specific information about the change (PR title, author, mergeability, etc.) that may be enriched after the record is first written. `UpdatedAt` reflects the last metadata change; `CreatedAt` is fixed at write time.
11+
- **Idempotent writes.** `Create` ignores primary-key conflicts so queue-redelivery of the same request is a safe no-op.
12+
- **No liveness filter.** `FindOverlapping` returns records regardless of whether the owning request is still in flight. Callers must check liveness against `RequestStore` themselves — the store boundary is intentionally one query, one table, no joins.
13+
- **Append-only by design.** Records are not deleted when the owning request reaches a terminal state; the historical claim is preserved for audit. Duplicate detection filters terminals out at query time via the controller-side liveness check.
14+
15+
## Implementing a Backend
16+
17+
1. Create `extension/changestore/{backend}/` directory.
18+
2. Implement the `ChangeStore` interface.
19+
3. Add schema files under `extension/changestore/{backend}/schema/` if the backend requires them.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package changestore
16+
17+
//go:generate mockgen -source=change_store.go -destination=mock/change_store_mock.go -package=mock
18+
19+
import (
20+
"context"
21+
22+
"github.com/uber/submitqueue/entity"
23+
)
24+
25+
// ChangeStore manages per-URI claim records for in-flight land requests.
26+
// Each row records that a specific URI was claimed by a specific request, scoped to a queue.
27+
// The (URI, RequestID) pair is the immutable identity of a record. Metadata may evolve over time.
28+
//
29+
// The store is the source of truth for "which URIs are or were associated with which requests".
30+
// Liveness of an owning request is NOT tracked here — callers must consult RequestStore separately
31+
// to determine whether an owner is in a terminal state.
32+
type ChangeStore interface {
33+
// Create persists a batch of ChangeRecords. Conflicts on the (URI, RequestID) primary key
34+
// are silently ignored, making the call idempotent under queue redeliveries of the same request.
35+
// Records belonging to different requests do not conflict — overlap between requests is detected
36+
// by FindOverlapping, not by Create.
37+
Create(ctx context.Context, records []entity.ChangeRecord) error
38+
39+
// FindOverlapping returns ChangeRecords whose URI is in the given set, scoped to queue,
40+
// excluding any records belonging to excludeRequestID (so callers can skip self when checking
41+
// for duplicates of a freshly-claimed request). Returns an empty slice when there is no overlap.
42+
//
43+
// Liveness of the returned records' owning requests is NOT filtered here — the caller is
44+
// responsible for consulting RequestStore to skip terminal owners.
45+
FindOverlapping(
46+
ctx context.Context,
47+
queue string,
48+
uris []string,
49+
excludeRequestID string,
50+
) ([]entity.ChangeRecord, error)
51+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
load("@rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "mock",
5+
srcs = ["change_store_mock.go"],
6+
importpath = "github.com/uber/submitqueue/extension/changestore/mock",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//entity",
10+
"@org_uber_go_mock//gomock",
11+
],
12+
)

extension/changestore/mock/change_store_mock.go

Lines changed: 71 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
load("@rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "mysql",
5+
srcs = ["change_store.go"],
6+
importpath = "github.com/uber/submitqueue/extension/changestore/mysql",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//core/metrics",
10+
"//entity",
11+
"//extension/changestore",
12+
"@com_github_uber_go_tally_v4//:tally",
13+
],
14+
)
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package mysql
16+
17+
import (
18+
"context"
19+
"database/sql"
20+
"fmt"
21+
"strings"
22+
23+
"github.com/uber-go/tally/v4"
24+
25+
"github.com/uber/submitqueue/core/metrics"
26+
"github.com/uber/submitqueue/entity"
27+
"github.com/uber/submitqueue/extension/changestore"
28+
)
29+
30+
type changeStore struct {
31+
db *sql.DB
32+
scope tally.Scope
33+
}
34+
35+
// NewChangeStore creates a new MySQL-backed ChangeStore.
36+
func NewChangeStore(db *sql.DB, scope tally.Scope) changestore.ChangeStore {
37+
return &changeStore{db: db, scope: scope}
38+
}
39+
40+
// Create inserts a batch of ChangeRecords. Primary-key conflicts on (uri, request_id)
41+
// are silently ignored via INSERT IGNORE so queue-redelivery of the same request is a no-op.
42+
func (s *changeStore) Create(ctx context.Context, records []entity.ChangeRecord) (retErr error) {
43+
op := metrics.Begin(s.scope, "create")
44+
defer func() { op.Complete(retErr) }()
45+
46+
if len(records) == 0 {
47+
return nil
48+
}
49+
50+
const cols = 6
51+
placeholders := strings.Repeat("(?, ?, ?, ?, ?, ?), ", len(records))
52+
placeholders = placeholders[:len(placeholders)-2] // trim trailing ", "
53+
54+
args := make([]any, 0, len(records)*cols)
55+
for _, r := range records {
56+
// Pass empty Metadata as NULL — JSON column rejects empty string but accepts NULL.
57+
var metadata any
58+
if r.Metadata != "" {
59+
metadata = r.Metadata
60+
}
61+
args = append(args, r.URI, r.RequestID, r.Queue, metadata, r.CreatedAt, r.UpdatedAt)
62+
}
63+
64+
query := "INSERT IGNORE INTO `change` (uri, request_id, queue, metadata, created_at, updated_at) VALUES " + placeholders
65+
if _, err := s.db.ExecContext(ctx, query, args...); err != nil {
66+
return fmt.Errorf("failed to insert change records (count=%d): %w", len(records), err)
67+
}
68+
return nil
69+
}
70+
71+
// FindOverlapping returns ChangeRecords whose uri is in the given set, scoped to queue,
72+
// excluding any belonging to excludeRequestID.
73+
func (s *changeStore) FindOverlapping(
74+
ctx context.Context,
75+
queue string,
76+
uris []string,
77+
excludeRequestID string,
78+
) (ret []entity.ChangeRecord, retErr error) {
79+
op := metrics.Begin(s.scope, "find_overlapping")
80+
defer func() { op.Complete(retErr) }()
81+
82+
if len(uris) == 0 {
83+
return nil, nil
84+
}
85+
86+
uriPlaceholders := "?" + strings.Repeat(", ?", len(uris)-1)
87+
query := "SELECT uri, request_id, queue, metadata, created_at, updated_at FROM `change` " +
88+
"WHERE uri IN (" + uriPlaceholders + ") AND queue = ? AND request_id != ?"
89+
90+
args := make([]any, 0, len(uris)+2)
91+
for _, u := range uris {
92+
args = append(args, u)
93+
}
94+
args = append(args, queue, excludeRequestID)
95+
96+
rows, err := s.db.QueryContext(ctx, query, args...)
97+
if err != nil {
98+
return nil, fmt.Errorf("failed to query overlapping changes for queue=%s: %w", queue, err)
99+
}
100+
defer rows.Close()
101+
102+
var results []entity.ChangeRecord
103+
for rows.Next() {
104+
var rec entity.ChangeRecord
105+
var metadata sql.NullString
106+
if err := rows.Scan(&rec.URI, &rec.RequestID, &rec.Queue, &metadata, &rec.CreatedAt, &rec.UpdatedAt); err != nil {
107+
return nil, fmt.Errorf("failed to scan change record for queue=%s: %w", queue, err)
108+
}
109+
if metadata.Valid {
110+
rec.Metadata = metadata.String
111+
}
112+
results = append(results, rec)
113+
}
114+
if err := rows.Err(); err != nil {
115+
return nil, fmt.Errorf("failed to iterate change records for queue=%s: %w", queue, err)
116+
}
117+
return results, nil
118+
}

0 commit comments

Comments
 (0)