Skip to content

Conversation

@morgando
Copy link
Contributor

@morgando morgando commented Oct 23, 2025

  • Runs full test suite under snapshot isolation: 4a16dfc
  • Updates expected output for many tests to reflect snapshot behavior: b971f5a

@morgando morgando force-pushed the modsnap_default branch 2 times, most recently from 053ccf8 to edb4d40 Compare October 23, 2025 19:56
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Error ⚠.
Cbuild submission: Success ✓.
Regression testing: 14/19 tests failed ⚠.

The first 10 failing tests are:
sc_force
socksql_master_swings

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Error ⚠.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
comdb2sys_queueodh_generated
socksql
tunables
reco-ddlk-sql

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Error ⚠.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
disttxn [core dumped]
analyze_fastinit_race
sc_force
socksql_master_swings
systable_locking
sc_resume_logicalsc_generated
sc_resume
queuedb_rollover_noroll1_generated
queuedb_rollover
sc_versmismatch_logicalsc_generated

@morgando morgando force-pushed the modsnap_default branch 4 times, most recently from d4611da to db54e9b Compare October 24, 2025 22:26
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Error ⚠.
Cbuild submission: Success ✓.
Regression testing: 526/627 tests failed ⚠.

The first 10 failing tests are:
disttxn [core dumped]
analyze_fastinit_race
sc_resume_logicalsc_generated
sc_resume
queuedb_rollover_noroll1_generated
queuedb_rollover
sc_versmismatch
sc_versmismatch_logicalsc_generated
recover_deadlock
prepare

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Error ⚠.
Cbuild submission: Success ✓.
Regression testing: 439/627 tests failed ⚠.

The first 10 failing tests are:
disttxn [core dumped]
tmptbl_leak [core dumped]
analyze_fastinit_race
sc_resume_logicalsc_generated
sc_resume
queuedb_rollover_noroll1_generated
queuedb_rollover
prepare
selectv_rcode_serialize_reads_like_writes_generated
udf

@morgando morgando force-pushed the modsnap_default branch 3 times, most recently from a58b9eb to ebb9048 Compare October 27, 2025 22:09
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Error ⚠.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
disttxn [core dumped]
tmptbl_leak_starve_generated [core dumped]
tmptbl_leak [core dumped]
analyze_fastinit_race
sc_resume_logicalsc_generated
sc_resume
prepare
selectv_rcode_serialretry_generated
selectv_rcode_disable_svonly_nop_generated
udf

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 145/200 tests failed ⚠.

The first 10 failing tests are:
cldeadlock [setup failure]
disttxn [core dumped]
newsqlreplay
random_osql_replay
sc_lotsoftables
pg_free_recovery
dohsql_race
insert_lots
insert_lots_ssl_generated
epochms_rollover

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
cldeadlock [setup failure]
disttxn [core dumped]
newsqlreplay
random_osql_replay
dohsql_race
queuedb_rollover_noroll1_generated
pg_free_recovery
sc_drop
queuedb_rollover
insert_lots

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Success ✓.
Cbuild submission: Error ⚠.
Regression testing: 71/630 tests failed ⚠.

The first 10 failing tests are:
ssl_san [setup failure]
disttxn [core dumped]
tmptbl_leak_starve_generated [core dumped]
tmptbl_leak [core dumped]
logdelete [core dumped]
schemalk_logicalsc_generated
schemalk_extralock_generated
schemalk
remotecreate_twopc_generated
remotecreate

@@ -1,7 +1,9 @@
(version='1')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont understand this test

@@ -1,3 +1,3 @@
(c1='one', comdb2_ctxinfo ( 'parallel' )=1, a=1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't understand this test

@morgando morgando force-pushed the modsnap_default branch 2 times, most recently from 5278b89 to 23b5ad2 Compare October 30, 2025 21:15
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 50/630 tests failed ⚠.

The first 10 failing tests are:
disttxn [core dumped]
tmptbl_leak [core dumped]
logdelete [core dumped]
logfill
analyze_fastinit_race
remotecreate_twopc_generated
remotecreate
phys_rep_tiered_firstfile_generated
sc_resume_logicalsc_generated
sc_resume

@morgando morgando force-pushed the modsnap_default branch 5 times, most recently from 5909dd4 to 48fa11f Compare November 4, 2025 15:50
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
disttxn [core dumped]
writes_remsql_names_partial_index_off_generated [core dumped]
schemalk
remotecreate_twopc_generated
remotecreate
sc_resume
sc_resume_logicalsc_generated
selectv_partial_index_off_generated
selectv
consumer_non_atomic_default_consumer_generated

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
disttxn [core dumped]
insert_lots_ssl_generated
insert_lots
schemalk
remotecreate
remotecreate_twopc_generated
sc_resume_logicalsc_generated
sc_resume
selectv_partial_index_off_generated
selectv

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style check: Success ✓.
Cbuild submission: Success ✓.
Regression testing: Success ✓.

Signed-off-by: mdouglas47 <mdouglas47@bloomberg.net>
Signed-off-by: mdouglas47 <mdouglas47@bloomberg.net>
Signed-off-by: mdouglas47 <mdouglas47@bloomberg.net>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so run this test in blocksql

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so run this test in blocksql

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so run this test in blocksql

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added explicit set transaction blocksql in places where this test expects to be running in blocksql

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so I set mode to blocksql before running chunk txns in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so I set mode to blocksql before running chunk txns in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test tests that a bunch of selects that run during schema changes succeed because they go through the recover ddlk process correctly. This is not relevant for snapshot since these selects should fail if we're running in snapshot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so I set mode to blocksql before running chunk txns in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so I set mode to blocksql before running chunk txns in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so I set mode to blocksql before running chunk txns in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so I set mode to blocksql before running chunk txns in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously want to run blocksql transactions for blocksql test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so run this test in blocksql

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so run this test in blocksql

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we're running snapshot by default, so do the insert by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing txn mode changes order of output here. Talked with rest of team about it and consensus was that this is okay. I'm forgetting the reasoning though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rows increase here because the txn can see its own updates.

Copy link
Contributor Author

@morgando morgando Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since snapshot is enabled by default, we can run snapshot transactions without enabling any tunables. Updated this test so that the select succeeds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chunk txns don't work in snapshot, so I set mode to blocksql before running chunk txns in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test says socksql in name, so set default mode accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transaction is

begin
select nextval('foo')
select curval('foo')
select nextval('foo')
commit

when we're running this test in snapshot isolation, select curval('foo') returns 11 instead of 12. Made a change to reflect this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants