logical: add the txnscheduler package by jeffswenson · Pull Request #164544 · cockroachdb/cockroach

jeffswenson · 2026-02-27T20:26:00Z

The txnscheduler takes transactions with replication locks and transforms them into a sequence of transactions with explicit cross-transaction dependency information.

Release note: none
Epic: CRDB-57649

trunk-io · 2026-02-27T20:26:04Z

😎 Merged successfully - details.

cockroach-teamcity · 2026-02-27T20:26:12Z

This change is

msbutler

Looks great! left mostly clarifying questions

msbutler · 2026-03-02T16:28:35Z

pkg/crosscluster/logical/txnscheduler/scheduler.go

+// Schedule requires the lock hashes within each transaction to be unique.
+// Duplicates must be filtered out at the lock synthesis phase.
+func (s *Scheduler) Schedule(
+	transaction Transaction, scratch []hlc.Timestamp,


nit: rename scratch to dependenciesBuffer so the caller understands its purpose?

pkg/crosscluster/logical/txnscheduler/scheduler.go

msbutler · 2026-03-02T21:40:39Z

pkg/crosscluster/logical/txnscheduler/scheduler.go

+	for _, lock := range txn.Locks {
+		locks, exists := s.lockMap[lock.Hash]
+		if !exists {
+			continue


you could add a little docstring here to explain why this is ok, or you could leave it as an exercise for the reader if you want.

This can happen if a subsequent write overwrote the lock table entry and committed before the earlier txn. For example:

t3 depends on t1 for key A

t2 depends on t1 for Key A

and t3 applies before t2, which will clean up the lock table for Key A before t2 applies.

I added a test only assertion here. This branch should never be taken because we don't remove applied transactions from the scheduler, we only ever advance the whole frontier as we need to reclaim memory.

ah, right. got my wires crossed here.

msbutler · 2026-03-02T22:05:07Z

pkg/crosscluster/logical/txnscheduler/lock_list.go

+	}
+	if head.readLocks == 0 {
+		nextHead := head.next
+		lt.locks[nextHead].writeLock = head.writeLock


i may be misreading things, but i don't understand how head could have a writeLock. Suppose we're removing read locks for txn1, we know:

txn1 is the oldest timestamp in the lock table

we know that newer write locks clear out older read locks for a given key when recorded

Therefore, when removing old read locks on a given key, we should not encounter any write locks, right?

Removed this and added a note.

msbutler · 2026-03-02T22:15:49Z

pkg/crosscluster/logical/txnscheduler/scheduler_benchmark_test.go

+func BenchmarkScheduler_Schedule(b *testing.B) {
+	for _, txnSize := range []int{1, 10, 50, 1000} {
+		b.Run(fmt.Sprintf("size=%d", txnSize), func(b *testing.B) {
+			transactions := createRandomTransactions(b.N/txnSize, txnSize, 10000, 0.8)


what have you been setting N to in your benchmarks? I hope is much larger than 10,000 the size of the keyspace, to ensure the benchmark exercises key collisions/long read lock chains.

I let go pick this, but it usually ends up being several million.

It ends up being several million.

msbutler · 2026-03-02T22:23:05Z

pkg/crosscluster/logical/txnscheduler/scheduler_test.go

+		time          int
+		readLocks     []int
+		writeLocks    []int
+		dependsOn     []int


nit: rename to expectedDependencies and expectedHorizon to make it clearer these validation fields.

pkg/crosscluster/logical/txnscheduler/scheduler_test.go

msbutler · 2026-03-02T22:40:56Z

pkg/crosscluster/logical/txnscheduler/scheduler_test.go

+	"github.com/stretchr/testify/require"
+)
+
+func TestScheduler(t *testing.T) {


it would be nice to build some sort of randomized scheduler test based on some DAG with read/write dependencies. This would ensure we have good coverage over lock_list.go. Not necessary for this PR.

Sorry, not a DAG: you could construct a 2D grid of discrete times and keys. Each entry in the grid is read lock, write lock or empty. You Could easily validate the schedule output with this I think.

I added a random test that uses a 2d oracle.

test looks great! If you haven't already done so, it could be worth sanity checking that the oracle deps look reasonable when table size is much lower.

msbutler · 2026-03-02T22:43:37Z

pkg/crosscluster/logical/txnscheduler/scheduler_test.go

+		for range rng.Intn(10) + 1 {
+			txn := makeTxn(false)
+
+			locks, horizon := scheduler.Schedule(txn, nil)


nit: rename locks, dependencies

msbutler

LGTM! just a few tiny nits

msbutler · 2026-03-04T13:45:59Z

pkg/crosscluster/logical/txnscheduler/scheduler.go

+	"github.com/cockroachdb/cockroach/pkg/util/ring"
+)
+
+// NewScheduler constructs a scheduler that can track at least `size` locks.


at most 'size' locks?

I updated this comment:

-// NewScheduler constructs a scheduler that can track at least `size` locks. +// NewScheduler constructs a scheduler that can track at most `size` lock table +// entries. Each lock table entry can track one write lock and up to 8 read +// locks.

msbutler · 2026-03-04T13:46:39Z

pkg/crosscluster/logical/txnscheduler/scheduler.go

+//
+// |            | Read Lock   | Write Lock |
+// |------------|-------------|------------|
+// | Read Lock  | Dependency  | Dependency |


no dependency with read x read

pkg/crosscluster/logical/txnscheduler/scheduler.go

msbutler · 2026-03-04T13:59:02Z

pkg/crosscluster/logical/txnscheduler/scheduler.go

+	for _, lock := range txn.Locks {
+		locks, exists := s.lockMap[lock.Hash]
+		if !exists {
+			continue


ah, right. got my wires crossed here.

msbutler · 2026-03-04T14:19:42Z

pkg/crosscluster/logical/txnscheduler/scheduler_test.go

+	"github.com/stretchr/testify/require"
+)
+
+func TestScheduler(t *testing.T) {


test looks great! If you haven't already done so, it could be worth sanity checking that the oracle deps look reasonable when table size is much lower.

The txnscheduler takes transactions with replication locks and transforms them into a sequence of transactions with explicit cross-transaction dependency information. Release note: none Epic: CRDB-57649

jeffswenson · 2026-03-04T16:44:23Z

Here's an example of what a small oracle table looks like:

    scheduler_test.go:234: numTransactions=4 numLocks=4
    scheduler_test.go:256:         txn0    txn1    txn2    txn3
    scheduler_test.go:262: lock0     -       R       -       R
    scheduler_test.go:262: lock1     R       R       -       -
    scheduler_test.go:262: lock2     R       -       W       R
    scheduler_test.go:262: lock3     -       R       -       -
    scheduler_test.go:267: txn0 deps: []
    scheduler_test.go:267: txn1 deps: []
    scheduler_test.go:267: txn2 deps: [0.000000001,0]
    scheduler_test.go:267: txn3 deps: [0.000000003,0]

I spot checked a few of these and they look correct.

jeffswenson · 2026-03-04T16:45:30Z

Thanks for the review!

/trunk merge

jeffswenson requested a review from msbutler February 27, 2026 20:26

jeffswenson requested a review from a team as a code owner February 27, 2026 20:26

jeffswenson requested review from andyyang890 and removed request for a team February 27, 2026 20:26

jeffswenson force-pushed the jeffswenson-txn-scheduler branch 2 times, most recently from 629217d to a5c6a74 Compare March 1, 2026 13:34

msbutler reviewed Mar 2, 2026

View reviewed changes

jeffswenson force-pushed the jeffswenson-txn-scheduler branch 3 times, most recently from d217687 to be7dd80 Compare March 3, 2026 21:45

jeffswenson requested a review from msbutler March 4, 2026 12:18

msbutler approved these changes Mar 4, 2026

View reviewed changes

jeffswenson force-pushed the jeffswenson-txn-scheduler branch from be7dd80 to ff7b772 Compare March 4, 2026 16:26

logical: add the txnscheduler package

4bd3f09

The txnscheduler takes transactions with replication locks and transforms them into a sequence of transactions with explicit cross-transaction dependency information. Release note: none Epic: CRDB-57649

jeffswenson force-pushed the jeffswenson-txn-scheduler branch from ff7b772 to 4bd3f09 Compare March 4, 2026 16:44

trunk-io bot merged commit 2c65768 into cockroachdb:master Mar 5, 2026
37 of 38 checks passed

celeste-cockroachdb bot added the target-release-26.2.0 label Mar 5, 2026

Conversation

jeffswenson commented Feb 27, 2026

Uh oh!

trunk-io bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Feb 27, 2026

Uh oh!

msbutler 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msbutler Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msbutler 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffswenson commented Mar 4, 2026

Uh oh!

jeffswenson commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

trunk-io bot commented Feb 27, 2026 •

edited

Loading

msbutler Mar 2, 2026 •

edited

Loading