Skip to content

Commit ff7e6f0

Browse files
authored
fix(tests): make replay and shard read setup deterministic (#26051)
## What type of PR is this? - [x] BUG - [x] Test and CI ## What this PR does / why we need it This PR fixes two independent nondeterministic test setups without relaxing production behavior. ### TAE checkpoint replay TestAppendAndGC2 created its database and tables through low-level TAE catalog APIs. Those APIs mutate the in-memory catalog but do not append rows to mo_database, mo_tables, and mo_columns. After a forced checkpoint truncates the DDL WAL, checkpoint replay can therefore lose the still-live database. The prepared-DML replay fence added by #26027 exposed this invalid setup as ExpectedEOB. The previous version of this PR skipped missing catalog entries, but diagnostics proved the missing IDs were the live test database and table. That production-code relaxation was removed. The test now uses the full DDL helpers and, after restart, opens both relations and scans every column to verify that each still contains all 99 rows. ### Shard read compatibility test TestReadValidatesAllRemoteShardsBeforeSending could create its table before all three CNs reported to the shard server. In the failed Ubuntu job, all shards were initially allocated to cn1 and balancing the second shard took 60 seconds. The shared setup context expires after 10 seconds, so the first remote compatibility check returned context deadline exceeded after one Adjust call instead of reaching the intended incompatible CN. The test now waits for every CN before creating shards, bounds every replica wait, uses separate setup and read contexts, and asserts the exact incompatibility error. This keeps the test fast and prevents unrelated timeout errors from satisfying require.Error. No production code is changed. ## Validation - TAE focused replay test: count=20 - TAE focused replay test: race count=10 - pkg/vm/engine/tae/db/test full package - pkg/vm/engine/tae/txn/txnimpl full package - shard compatibility test: count=30 - shard compatibility test: race count=10 - pkg/shardservice full package - go build ./pkg/shardservice - go vet ./pkg/shardservice - git diff --check
1 parent 65af00d commit ff7e6f0

2 files changed

Lines changed: 40 additions & 11 deletions

File tree

pkg/shardservice/service_read_test.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,23 @@ func TestReadValidatesAllRemoteShardsBeforeSending(t *testing.T) {
208208
runServicesTest(
209209
t,
210210
"cn1,cn2,cn3",
211-
func(ctx context.Context, _ *server, services []*service) {
211+
func(_ context.Context, server *server, services []*service) {
212212
s := services[0]
213213
table := uint64(1)
214-
mustAddTestShards(t, ctx, s, table, 3, 1, services[1:]...)
214+
require.Eventually(t, func() bool {
215+
server.r.RLock()
216+
defer server.r.RUnlock()
217+
return len(server.r.cns) == len(services)
218+
}, 10*time.Second, 10*time.Millisecond)
219+
func() {
220+
setupCtx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
221+
defer cancel()
222+
mustAddTestShards(t, setupCtx, s, table, 3, 1, services[1:]...)
223+
}()
215224
for _, service := range services {
216-
waitReplicaCount(table, service, 1)
225+
require.Eventually(t, func() bool {
226+
return service.TableReplicaCount(table) == 1
227+
}, 10*time.Second, 10*time.Millisecond)
217228
}
218229

219230
cache, err := s.getShards(table)
@@ -253,8 +264,10 @@ func TestReadValidatesAllRemoteShardsBeforeSending(t *testing.T) {
253264
client := &countingMethodBasedClient{MethodBasedClient: s.remote.client}
254265
s.remote.client = client
255266
adjustCalls := make(map[uint64]int)
267+
readCtx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
268+
defer cancel()
256269
err = s.Read(
257-
ctx,
270+
readCtx,
258271
ReadRequest{
259272
TableID: table,
260273
Param: shard.ReadParam{Process: pipeline.ProcessInfo{
@@ -266,6 +279,7 @@ func TestReadValidatesAllRemoteShardsBeforeSending(t *testing.T) {
266279
}),
267280
)
268281
require.Error(t, err)
282+
require.ErrorContains(t, err, "incompatible or unknown commit")
269283
require.Zero(t, client.asyncCalls.Load())
270284
require.Len(t, adjustCalls, len(remoteTargets))
271285
for _, calls := range adjustCalls {

pkg/vm/engine/tae/db/test/db_test.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7077,13 +7077,13 @@ func TestAppendAndGC2(t *testing.T) {
70777077
schema2.Extra.ObjectMaxBlocks = 2
70787078
{
70797079
txn, _ := db.StartTxn(nil)
7080-
database, err := txn.CreateDatabase("db", "", "")
7081-
assert.Nil(t, err)
7082-
_, err = database.CreateRelation(schema1)
7083-
assert.Nil(t, err)
7084-
_, err = database.CreateRelation(schema2)
7085-
assert.Nil(t, err)
7086-
assert.Nil(t, txn.Commit(context.Background()))
7080+
database, err := testutil.CreateDatabase2(ctx, txn, "db")
7081+
require.NoError(t, err)
7082+
_, err = testutil.CreateRelation2(ctx, txn, database, schema1)
7083+
require.NoError(t, err)
7084+
_, err = testutil.CreateRelation2(ctx, txn, database, schema2)
7085+
require.NoError(t, err)
7086+
require.NoError(t, txn.Commit(ctx))
70877087
}
70887088
bat := catalog.MockBatch(schema1, int(schema1.Extra.BlockMaxRows*10-1))
70897089
defer bat.Close()
@@ -7110,6 +7110,21 @@ func TestAppendAndGC2(t *testing.T) {
71107110
metaFile := db.BGCheckpointRunner.GetCheckpointMetaFiles()
71117111
tae.Restart(ctx)
71127112
db = tae.DB
7113+
func() {
7114+
replayTxn, err := db.StartTxn(nil)
7115+
require.NoError(t, err)
7116+
defer func() {
7117+
require.NoError(t, replayTxn.Rollback(ctx))
7118+
}()
7119+
replayDB, err := replayTxn.GetDatabase("db")
7120+
require.NoError(t, err)
7121+
replayRel1, err := replayDB.GetRelationByName(schema1.Name)
7122+
require.NoError(t, err)
7123+
replayRel2, err := replayDB.GetRelationByName(schema2.Name)
7124+
require.NoError(t, err)
7125+
testutil.CheckAllColRowsByScan(t, replayRel1, bat.Length(), false)
7126+
testutil.CheckAllColRowsByScan(t, replayRel2, bat.Length(), false)
7127+
}()
71137128
files := make(map[string]struct{}, 0)
71147129
loadFiles := func(group uint32, lsn uint64, payload []byte, typ uint16, info any) driver.ReplayEntryState {
71157130
if group != wal.GroupFiles {

0 commit comments

Comments
 (0)