Skip to content

Commit

Permalink
kvserver: disable old closed timestamps mechanism
Browse files Browse the repository at this point in the history
When the new one is enabled, the old one will now be disabled. No point
in running with both of them.

Release note: None
Release justification: Needed for GLOBAL tables.
  • Loading branch information
andreimatei committed Mar 2, 2021
1 parent f7edbe4 commit 5d1f1f3
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 0 deletions.
8 changes: 8 additions & 0 deletions pkg/kv/kvserver/closedts/closedts.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/hlc"
)

// IssueTrackingRemovalOfOldClosedTimestampsCode is the Github issue tracking
// the deletion of the "old" closed timestamps code (i.e. everything around
// here) in 21.2, now that 21.1 has a new Raft-based closed-timestamps
// mechanism. The old mechanism is disabled when the cluster version is
// sufficiently high, and all the tests failing because of it are skipped with
// this issue.
const IssueTrackingRemovalOfOldClosedTimestampsCode = 61299

// ReleaseFunc is a closure returned from Track which is used to record the
// LeaseAppliedIndex (LAI) given to a tracked proposal. The supplied epoch must
// match that of the lease under which the proposal was proposed.
Expand Down
2 changes: 2 additions & 0 deletions pkg/kv/kvserver/closedts/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/stop"
Expand Down Expand Up @@ -129,6 +130,7 @@ func setupTwoNodeTest() (_ *TestContainer, _ *TestContainer, shutdown func()) {

func TestTwoNodes(t *testing.T) {
defer leaktest.AfterTest(t)()
skip.WithIssue(t, closedts.IssueTrackingRemovalOfOldClosedTimestampsCode)

ctx := context.Background()

Expand Down
7 changes: 7 additions & 0 deletions pkg/kv/kvserver/closedts/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"sync"
"time"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/ctpb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
Expand Down Expand Up @@ -129,6 +130,12 @@ func (p *Provider) runCloser(ctx context.Context) {
var t timeutil.Timer
defer t.Stop()
for {
// If the "new" closed timestamps mechanism is enabled, we inhibit this old one.
if p.cfg.Settings.Version.IsActive(ctx, clusterversion.ClosedTimestampsRaftTransport) {
log.Infof(ctx, "disabling legacy closed-timestamp mechanism; the new one is enabled")
break
}

closeFraction := closedts.CloseFraction.Get(&p.cfg.Settings.SV)
targetDuration := float64(closedts.TargetDuration.Get(&p.cfg.Settings.SV))
if targetDuration > 0 {
Expand Down
3 changes: 3 additions & 0 deletions pkg/kv/kvserver/closedts/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand All @@ -37,6 +38,7 @@ import (

func TestProviderSubscribeNotify(t *testing.T) {
defer leaktest.AfterTest(t)()
skip.WithIssue(t, closedts.IssueTrackingRemovalOfOldClosedTimestampsCode)

ctx := context.Background()
stopper := stop.NewStopper()
Expand Down Expand Up @@ -293,6 +295,7 @@ func TestProviderSubscribeConcurrent(t *testing.T) {
// value re-enables it.
func TestProviderTargetDurationSetting(t *testing.T) {
defer leaktest.AfterTest(t)()
skip.WithIssue(t, closedts.IssueTrackingRemovalOfOldClosedTimestampsCode)

st := cluster.MakeTestingClusterSettings()
closedts.TargetDuration.Override(&st.SV, time.Millisecond)
Expand Down
6 changes: 6 additions & 0 deletions pkg/kv/kvserver/closedts/transport/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"context"
"unsafe"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/ctpb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
Expand Down Expand Up @@ -66,6 +67,11 @@ type client struct {
// done so, the information should soon thereafter be available to the Sink and
// from there, further follower read attempts. Does not block.
func (pr *Clients) Request(nodeID roachpb.NodeID, rangeID roachpb.RangeID) {
// If the new closed timestamps is enabled, this old one is disabled.
if pr.cfg.Settings.Version.IsActive(context.TODO(), clusterversion.ClosedTimestampsRaftTransport) {
return
}

if nodeID == pr.cfg.NodeID {
return
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/kv/kvserver/closedts/transport/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ import (
"strings"
"testing"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/ctpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/transport"
transporttestutils "github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/transport/testutils"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/stop"
Expand Down Expand Up @@ -76,6 +78,7 @@ func assertNumSubscribers(t *testing.T, p *TestProducer, exp int) {

func TestTransportConnectOnRequest(t *testing.T) {
defer leaktest.AfterTest(t)()
skip.WithIssue(t, closedts.IssueTrackingRemovalOfOldClosedTimestampsCode)

container := NewTestContainer()
defer container.Stopper.Stop(context.Background())
Expand Down Expand Up @@ -108,6 +111,7 @@ func TestTransportConnectOnRequest(t *testing.T) {

func TestTransportClientReceivesEntries(t *testing.T) {
defer leaktest.AfterTest(t)()
skip.WithIssue(t, closedts.IssueTrackingRemovalOfOldClosedTimestampsCode)

container := NewTestContainer()
defer container.Stopper.Stop(context.Background())
Expand Down

0 comments on commit 5d1f1f3

Please sign in to comment.