Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 124 additions & 10 deletions client/clients/gc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
"math"
"time"

"github.com/pingcap/errors"
)

// Client is the interface for GC client.
Expand All @@ -32,6 +34,45 @@ type Client interface {
GetGCStatesClient(keyspaceID uint32) GCStatesClient
}

// GCStatesAPIOptions represents all options for GC states API.
//
//nolint:revive
type GCStatesAPIOptions struct {
ExcludeGCBarriers bool
ExcludeGlobalGCBarriers bool
}

// DefaultGCStatesAPIOptions returns the default options for GC states API.
func DefaultGCStatesAPIOptions() GCStatesAPIOptions {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This changes the Go client default behavior: existing callers of GetGCState/GetAllKeyspacesGCStates will silently stop receiving barriers. Can we keep the old default and make exclusion opt-in, or call this out as a breaking change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As it's hard to push all callers to update their usage, I suggest let this be a breaking change. Callers that really needs the GC barriers will receive compile error, and is forced to switch to the safe retriver function; then if they missed the new options, they will get an error.
Actually, I expect that there's no such usage for now. If so, nothing will happen after upgrading the PD client.

return GCStatesAPIOptions{
ExcludeGCBarriers: true,
ExcludeGlobalGCBarriers: true,
}
}

// GCStatesAPIOption is the type of option for GC states API.
//
//nolint:revive
type GCStatesAPIOption func(*GCStatesAPIOptions)

// ExcludeGCBarriers controls whether GetGCState and GetAllKeyspacesGCStates should exclude GC barriers.
// Enabling this can reduce the cost of reading GC states, and is recommended for most use cases.
// When GC barriers are needed, explicitly set false to this option.
func ExcludeGCBarriers(whetherToExclude bool) GCStatesAPIOption {
return func(opts *GCStatesAPIOptions) {
opts.ExcludeGCBarriers = whetherToExclude
}
}

// ExcludeGlobalGCBarriers controls whether GetAllKeyspacesGCStates should exclude global GC barriers.
// Enabling this can reduce the cost of reading GC states, and is recommended for most use cases.
// When global GC barriers are needed, explicitly set false to this option.
func ExcludeGlobalGCBarriers(whetherToExclude bool) GCStatesAPIOption {
return func(opts *GCStatesAPIOptions) {
opts.ExcludeGlobalGCBarriers = whetherToExclude
}
}

// GCStatesClient is the interface for users to access GC states.
// KeyspaceID is already bound to this type when created.
//
Expand Down Expand Up @@ -72,7 +113,7 @@ type GCStatesClient interface {
//
// When this method is called on a keyspace without keyspace-level GC enabled, it will be equivalent to calling it on
// the NullKeyspace.
GetGCState(ctx context.Context) (GCState, error)
GetGCState(ctx context.Context, opts ...GCStatesAPIOption) (GCState, error)
// SetGlobalGCBarrier sets a global GC barrier, which blocks GC like how GC barriers do, but is effective for all
// keyspaces. This API is designed for some special needs to block GC of all keyspaces.
//
Expand Down Expand Up @@ -105,11 +146,11 @@ type GCStatesClient interface {
SetGlobalGCBarrier(ctx context.Context, barrierID string, barrierTS uint64, ttl time.Duration) (*GlobalGCBarrierInfo, error)
// DeleteGlobalGCBarrier deletes a global GC barrier.
DeleteGlobalGCBarrier(ctx context.Context, barrierID string) (*GlobalGCBarrierInfo, error)
// Get the GC states from all keyspaces.
// GetAllKeyspacesGCStates gets GC states from all keyspaces.
// The return value includes both GC states and global GC barriers information.
// If a keyspace's state is not ENABLED(like DISABLE/ARCHIVED/TOMBSTONE), that keyspace is skipped.
// If a keyspace is not configured with keyspace level GC, its GCState data is missing.
GetAllKeyspacesGCStates(ctx context.Context) (ClusterGCStates, error)
GetAllKeyspacesGCStates(ctx context.Context, opts ...GCStatesAPIOption) (ClusterGCStates, error)
}

// InternalController is the interface for controlling GC execution.
Expand Down Expand Up @@ -245,16 +286,89 @@ func (b *GlobalGCBarrierInfo) isExpiredImpl(now time.Time) bool {
//nolint:revive
type GCState struct {
// The ID of the keyspace this GC state belongs to.
KeyspaceID uint32
TxnSafePoint uint64
GCSafePoint uint64
GCBarriers []*GCBarrierInfo
KeyspaceID uint32
TxnSafePoint uint64
GCSafePoint uint64
hasGCBarriers bool
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Removing the exported barrier fields breaks source compatibility for existing users. Could we keep exported fields for compatibility and add Has/Get helpers to distinguish not-fetched from an empty result?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ditto. And this prevents misuse.

gcBarriers []*GCBarrierInfo
}

// NewGCStateWithoutGCBarriers creates a GCState instance without GC barriers info.
func NewGCStateWithoutGCBarriers(keyspaceID uint32, txnSafePoint uint64, gcSafePoint uint64) GCState {
return GCState{
KeyspaceID: keyspaceID,
TxnSafePoint: txnSafePoint,
GCSafePoint: gcSafePoint,
hasGCBarriers: false,
gcBarriers: nil,
}
}

// NewGCStateWithGCBarriers creates a GCState instance with GC barriers info.
func NewGCStateWithGCBarriers(keyspaceID uint32, txnSafePoint uint64, gcSafePoint uint64, gcBarriers []*GCBarrierInfo) GCState {
return GCState{
KeyspaceID: keyspaceID,
TxnSafePoint: txnSafePoint,
GCSafePoint: gcSafePoint,
hasGCBarriers: true,
gcBarriers: gcBarriers,
}
}

// HasGCBarriers returns whether the GCState instance carries GC barriers info. Note that valid GC barriers info
// can be empty.
func (s GCState) HasGCBarriers() bool {
return s.hasGCBarriers
}

// GetGCBarriers retrieves GC barriers from the GCState instance, or returns an error if it doesn't carry any GC barrier
// info.
func (s GCState) GetGCBarriers() ([]*GCBarrierInfo, error) {
if !s.HasGCBarriers() {
return nil, errors.New("trying to get GC barriers from GCState that doesn't provide GC barriers info. " +
"to retrieve GC barriers, pass false to excludeGCBarriers parameter to GC APIs")
}
return s.gcBarriers, nil
}

// ClusterGCStates represents the information of the GC state for all keyspaces.
type ClusterGCStates struct {
// Maps from keyspace id to GC state of that keyspace.
GCStates map[uint32]GCState
// All existing global GC barriers.
GlobalGCBarriers []*GlobalGCBarrierInfo
GCStates map[uint32]GCState
hasGlobalGCBarriers bool
globalGCBarriers []*GlobalGCBarrierInfo
}

// NewClusterGCStatesWithoutGlobalGCBarriers creates a ClusterGCStates instance without global GC barriers info.
func NewClusterGCStatesWithoutGlobalGCBarriers(gcStates map[uint32]GCState) ClusterGCStates {
return ClusterGCStates{
GCStates: gcStates,
hasGlobalGCBarriers: false,
globalGCBarriers: nil,
}
}

// NewClusterGCStatesWithGlobalGCBarriers creates a ClusterGCStates instance with global GC barriers info.
func NewClusterGCStatesWithGlobalGCBarriers(gcStates map[uint32]GCState, globalGCBarriers []*GlobalGCBarrierInfo) ClusterGCStates {
return ClusterGCStates{
GCStates: gcStates,
hasGlobalGCBarriers: true,
globalGCBarriers: globalGCBarriers,
}
}

// HasGlobalGCBarriers returns whether the ClusterGCStates instance carries global GC barriers info. Note that valid
// global GC barriers info can be empty.
func (s ClusterGCStates) HasGlobalGCBarriers() bool {
return s.hasGlobalGCBarriers
}

// GetGlobalGCBarriers retrieves global GC barriers from the ClusterGCStates instance, or returns an error if it doesn't
// carry any global GC barrier info.
func (s ClusterGCStates) GetGlobalGCBarriers() ([]*GlobalGCBarrierInfo, error) {
if !s.HasGlobalGCBarriers() {
return nil, errors.New("trying to get global GC barriers from ClusterGCStates that doesn't provide global GC barriers info. " +
"to retrieve global GC barriers, pass false to excludeGlobalGCBarriers parameter to GC APIs")
}
return s.globalGCBarriers, nil
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
39 changes: 39 additions & 0 deletions client/clients/gc/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,42 @@ func TestGCBarrierInfoExpiration(t *testing.T) {
re.False(b1.isExpiredImpl(now))
re.False(b1.isExpiredImpl(now.Add(time.Hour * 24 * 365 * 10)))
}

func TestGCStateAccessors(t *testing.T) {
re := require.New(t)

state := NewGCStateWithoutGCBarriers(1, 2, 3)
re.False(state.HasGCBarriers())
barriers, err := state.GetGCBarriers()
re.Error(err)
re.Nil(barriers)

state = NewGCStateWithGCBarriers(1, 2, 3, []*GCBarrierInfo{
NewGCBarrierInfo("b1", 4, time.Second, time.Now()),
})
re.True(state.HasGCBarriers())
barriers, err = state.GetGCBarriers()
re.NoError(err)
re.Len(barriers, 1)
re.Equal("b1", barriers[0].BarrierID)
}

func TestClusterGCStatesAccessors(t *testing.T) {
re := require.New(t)

state := NewGCStateWithoutGCBarriers(1, 2, 3)
clusterState := NewClusterGCStatesWithoutGlobalGCBarriers(map[uint32]GCState{1: state})
re.False(clusterState.HasGlobalGCBarriers())
barriers, err := clusterState.GetGlobalGCBarriers()
re.Error(err)
re.Nil(barriers)

clusterState = NewClusterGCStatesWithGlobalGCBarriers(map[uint32]GCState{1: state}, []*GlobalGCBarrierInfo{
NewGlobalGCBarrierInfo("b1", 4, time.Second, time.Now()),
})
re.True(clusterState.HasGlobalGCBarriers())
barriers, err = clusterState.GetGlobalGCBarriers()
re.NoError(err)
re.Len(barriers, 1)
re.Equal("b1", barriers[0].BarrierID)
}
49 changes: 32 additions & 17 deletions client/gc_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,19 +295,25 @@ func (c gcStatesClient) DeleteGCBarrier(ctx context.Context, barrierID string) (
}

// GetGCState gets the current GC state.
func (c gcStatesClient) GetGCState(ctx context.Context) (gc.GCState, error) {
func (c gcStatesClient) GetGCState(ctx context.Context, opts ...gc.GCStatesAPIOption) (gc.GCState, error) {
if span := opentracing.SpanFromContext(ctx); span != nil && span.Tracer() != nil {
span = span.Tracer().StartSpan("pdclient.GetGCState", opentracing.ChildOf(span.Context()))
defer span.Finish()
}
start := time.Now()
defer func() { metrics.CmdDurationGetGCState.Observe(time.Since(start).Seconds()) }()

options := gc.DefaultGCStatesAPIOptions()
for _, opt := range opts {
opt(&options)
}

ctx, cancel := context.WithTimeout(ctx, c.client.inner.option.Timeout)
defer cancel()
req := &pdpb.GetGCStateRequest{
Header: c.client.requestHeader(),
KeyspaceScope: wrapKeyspaceScope(c.keyspaceID),
Header: c.client.requestHeader(),
KeyspaceScope: wrapKeyspaceScope(c.keyspaceID),
ExcludeGcBarriers: options.ExcludeGCBarriers,
}
protoClient, ctx := c.client.getClientAndContext(ctx)
if protoClient == nil {
Expand All @@ -319,10 +325,10 @@ func (c gcStatesClient) GetGCState(ctx context.Context) (gc.GCState, error) {
}

gcState := resp.GetGcState()
return pbToGCState(gcState, start), nil
return pbToGCState(gcState, start, options.ExcludeGCBarriers), nil
}

func pbToGCState(pb *pdpb.GCState, reqStartTime time.Time) gc.GCState {
func pbToGCState(pb *pdpb.GCState, reqStartTime time.Time, excludeGCBarriers bool) gc.GCState {
keyspaceID := constants.NullKeyspaceID
if pb.KeyspaceScope != nil {
keyspaceID = pb.KeyspaceScope.KeyspaceId
Expand All @@ -331,12 +337,10 @@ func pbToGCState(pb *pdpb.GCState, reqStartTime time.Time) gc.GCState {
for _, b := range pb.GetGcBarriers() {
gcBarriers = append(gcBarriers, pbToGCBarrierInfo(b, reqStartTime))
}
return gc.GCState{
KeyspaceID: keyspaceID,
TxnSafePoint: pb.GetTxnSafePoint(),
GCSafePoint: pb.GetGcSafePoint(),
GCBarriers: gcBarriers,
if !excludeGCBarriers {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Before this branch, we already allocate and convert all returned barriers. During rolling upgrades, an old PD may still return barriers even when the new client requested exclusion, so we can return early before the conversion.

return gc.NewGCStateWithGCBarriers(keyspaceID, pb.GetTxnSafePoint(), pb.GetGcSafePoint(), gcBarriers)
}
return gc.NewGCStateWithoutGCBarriers(keyspaceID, pb.GetTxnSafePoint(), pb.GetGcSafePoint())
}

// SetGlobalGCBarrier sets (creates or updates) a global GC barrier.
Expand Down Expand Up @@ -394,18 +398,25 @@ func (c gcStatesClient) DeleteGlobalGCBarrier(ctx context.Context, barrierID str
}

// GetAllKeyspacesGCStates gets the GC states from all keyspaces.
func (c gcStatesClient) GetAllKeyspacesGCStates(ctx context.Context) (gc.ClusterGCStates, error) {
func (c gcStatesClient) GetAllKeyspacesGCStates(ctx context.Context, opts ...gc.GCStatesAPIOption) (gc.ClusterGCStates, error) {
if span := opentracing.SpanFromContext(ctx); span != nil && span.Tracer() != nil {
span = span.Tracer().StartSpan("pdclient.GetAllKeyspacesGCState", opentracing.ChildOf(span.Context()))
defer span.Finish()
}
start := time.Now()
defer func() { metrics.CmdDurationGetAllKeyspacesGCStates.Observe(time.Since(start).Seconds()) }()

options := gc.DefaultGCStatesAPIOptions()
for _, opt := range opts {
opt(&options)
}

ctx, cancel := context.WithTimeout(ctx, c.client.inner.option.Timeout)
defer cancel()
req := &pdpb.GetAllKeyspacesGCStatesRequest{
Header: c.client.requestHeader(),
Header: c.client.requestHeader(),
ExcludeGcBarriers: options.ExcludeGCBarriers,
ExcludeGlobalGcBarriers: options.ExcludeGlobalGCBarriers,
}
protoClient, ctx := c.client.getClientAndContext(ctx)
if protoClient == nil {
Expand All @@ -417,19 +428,23 @@ func (c gcStatesClient) GetAllKeyspacesGCStates(ctx context.Context) (gc.Cluster
return gc.ClusterGCStates{}, err
}

var ret gc.ClusterGCStates
ret.GCStates = make(map[uint32]gc.GCState, len(resp.GetGcStates()))
gcStates := make(map[uint32]gc.GCState, len(resp.GetGcStates()))
for _, state := range resp.GetGcStates() {
var keyspaceID uint32
if state.KeyspaceScope == nil {
keyspaceID = constants.NullKeyspaceID
} else {
keyspaceID = state.KeyspaceScope.KeyspaceId
}
ret.GCStates[keyspaceID] = pbToGCState(state, start)
gcStates[keyspaceID] = pbToGCState(state, start, options.ExcludeGCBarriers)
}
if options.ExcludeGlobalGCBarriers {
return gc.NewClusterGCStatesWithoutGlobalGCBarriers(gcStates), nil
}

globalGCBarriers := make([]*gc.GlobalGCBarrierInfo, 0, len(resp.GetGlobalGcBarriers()))
for _, barrier := range resp.GetGlobalGcBarriers() {
ret.GlobalGCBarriers = append(ret.GlobalGCBarriers, pbToGlobalGCBarrierInfo(barrier, start))
globalGCBarriers = append(globalGCBarriers, pbToGlobalGCBarrierInfo(barrier, start))
}
return ret, nil
return gc.NewClusterGCStatesWithGlobalGCBarriers(gcStates, globalGCBarriers), nil
}
2 changes: 1 addition & 1 deletion client/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/opentracing/opentracing-go v1.2.0
github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c
github.com/pingcap/failpoint v0.0.0-20240528011301-b51a646c7c86
github.com/pingcap/kvproto v0.0.0-20260511034003-fc9e0458a359
github.com/pingcap/kvproto v0.0.0-20260514102340-daa7c864b473
github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3
github.com/prometheus/client_golang v1.20.5
github.com/stretchr/testify v1.9.0
Expand Down
4 changes: 2 additions & 2 deletions client/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c h1:xpW9bvK+HuuTm
github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c/go.mod h1:X2r9ueLEUZgtx2cIogM0v4Zj5uvvzhuuiu7Pn8HzMPg=
github.com/pingcap/failpoint v0.0.0-20240528011301-b51a646c7c86 h1:tdMsjOqUR7YXHoBitzdebTvOjs/swniBTOLy5XiMtuE=
github.com/pingcap/failpoint v0.0.0-20240528011301-b51a646c7c86/go.mod h1:exzhVYca3WRtd6gclGNErRWb1qEgff3LYta0LvRmON4=
github.com/pingcap/kvproto v0.0.0-20260511034003-fc9e0458a359 h1:oteLtLuoWZN3uvfH836U0IIJ+s3UOk11q7GaQ0Tk+wc=
github.com/pingcap/kvproto v0.0.0-20260511034003-fc9e0458a359/go.mod h1:z6+aAHB7dBkA+LyinEX+48/ImRJ3jag0Hg0c7wkhEvE=
github.com/pingcap/kvproto v0.0.0-20260514102340-daa7c864b473 h1:n6QWAac97mv2NJhn17iFPFnsE5fMgtPLNmsGZeqq78o=
github.com/pingcap/kvproto v0.0.0-20260514102340-daa7c864b473/go.mod h1:z6+aAHB7dBkA+LyinEX+48/ImRJ3jag0Hg0c7wkhEvE=
github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 h1:HR/ylkkLmGdSSDaD8IDP+SZrdhV1Kibl9KrHxJ9eciw=
github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3/go.mod h1:DWQW5jICDR7UJh4HtxXSM20Churx4CQL0fwL/SoOSA4=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ require (
github.com/pingcap/errcode v0.3.0
github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c
github.com/pingcap/failpoint v0.0.0-20240528011301-b51a646c7c86
github.com/pingcap/kvproto v0.0.0-20260511034003-fc9e0458a359
github.com/pingcap/kvproto v0.0.0-20260514102340-daa7c864b473
github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3
github.com/pingcap/metering_sdk v0.0.0-20260203082503-b9f282339654
github.com/pingcap/sysutil v1.0.1-0.20230407040306-fb007c5aff21
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,8 @@ github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c/go.mod h1:X2r9ue
github.com/pingcap/failpoint v0.0.0-20240528011301-b51a646c7c86 h1:tdMsjOqUR7YXHoBitzdebTvOjs/swniBTOLy5XiMtuE=
github.com/pingcap/failpoint v0.0.0-20240528011301-b51a646c7c86/go.mod h1:exzhVYca3WRtd6gclGNErRWb1qEgff3LYta0LvRmON4=
github.com/pingcap/kvproto v0.0.0-20191211054548-3c6b38ea5107/go.mod h1:WWLmULLO7l8IOcQG+t+ItJ3fEcrL5FxF0Wu+HrMy26w=
github.com/pingcap/kvproto v0.0.0-20260511034003-fc9e0458a359 h1:oteLtLuoWZN3uvfH836U0IIJ+s3UOk11q7GaQ0Tk+wc=
github.com/pingcap/kvproto v0.0.0-20260511034003-fc9e0458a359/go.mod h1:z6+aAHB7dBkA+LyinEX+48/ImRJ3jag0Hg0c7wkhEvE=
github.com/pingcap/kvproto v0.0.0-20260514102340-daa7c864b473 h1:n6QWAac97mv2NJhn17iFPFnsE5fMgtPLNmsGZeqq78o=
github.com/pingcap/kvproto v0.0.0-20260514102340-daa7c864b473/go.mod h1:z6+aAHB7dBkA+LyinEX+48/ImRJ3jag0Hg0c7wkhEvE=
github.com/pingcap/log v0.0.0-20210625125904-98ed8e2eb1c7/go.mod h1:8AanEdAHATuRurdGxZXBz0At+9avep+ub7U1AGYLIMM=
github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 h1:HR/ylkkLmGdSSDaD8IDP+SZrdhV1Kibl9KrHxJ9eciw=
github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3/go.mod h1:DWQW5jICDR7UJh4HtxXSM20Churx4CQL0fwL/SoOSA4=
Expand Down
Loading
Loading