Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
121495: rangefeed: don't set default txn push interval when pusher nil r=miraradeva a=kvoli

In 8eb7ca9, we reverted the change to remove the legacy rangefeed processor but omitted updating the `SetDefaults` config function. In-between removing the legacy rangefeed processor and subsequently reverting its removal, we updated the `SetDefaults` function to always set the default txn push interval (6fcbdfc).

This causes a NPE when dereferencing the txn pusher, which occurs during some benchmark testing.

Re-introduce the txn pusher nil check when setting the default processor push txn interval.

Fixes: cockroachdb#121429
Release note: None

121519: catalog/lease: fix flake in TestNameCacheEntryDoesntReturnExpiredLease r=fqazi a=fqazi

Previously, TestNameCacheEntryDoesntReturnExpiredLease would flake because of the automatic stats collection would periodically end up renewing the lease on our table after a fake expiration. To address this, this patch disables stats collection for this test.

Fixes: cockroachdb#121314

Release note: None

Co-authored-by: Austen McClernon <austen@cockroachlabs.com>
Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com>
  • Loading branch information
3 people committed Apr 2, 2024
3 parents 93b9ecc + a357a5d + 36feae3 commit ada3060
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
21 changes: 16 additions & 5 deletions pkg/kv/kvserver/rangefeed/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,22 @@ type Config struct {
// SetDefaults initializes unset fields in Config to values
// suitable for use by a Processor.
func (sc *Config) SetDefaults() {
if sc.PushTxnsInterval == 0 {
sc.PushTxnsInterval = DefaultPushTxnsInterval
}
if sc.PushTxnsAge == 0 {
sc.PushTxnsAge = defaultPushTxnsAge
// Some tests don't set the TxnPusher, so we avoid setting a default push txn
// interval in such cases #121429.
if sc.TxnPusher == nil {
if sc.PushTxnsInterval != 0 {
panic("nil TxnPusher with non-zero PushTxnsInterval")
}
if sc.PushTxnsAge != 0 {
panic("nil TxnPusher with non-zero PushTxnsAge")
}
} else {
if sc.PushTxnsInterval == 0 {
sc.PushTxnsInterval = DefaultPushTxnsInterval
}
if sc.PushTxnsAge == 0 {
sc.PushTxnsAge = defaultPushTxnsAge
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/catalog/lease/lease_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,11 @@ CREATE TABLE t.%s (k CHAR PRIMARY KEY, v CHAR);
t.Fatal(err)
}

// Disable stats collection so that the descriptor isn't modified.
if _, err := db.Exec("SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false"); err != nil {
t.Fatal(err)
}

// Populate the name cache.
if _, err := db.Exec("SELECT * FROM t.test;"); err != nil {
t.Fatal(err)
Expand Down

0 comments on commit ada3060

Please sign in to comment.