Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
19 changes: 15 additions & 4 deletions client/http/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ type Client interface {

/* Keyspace interface */

// UpdateKeyspaceConfig patches the keyspace config.
Comment thread
ystaticy marked this conversation as resolved.
Outdated
UpdateKeyspaceConfig(ctx context.Context, keyspaceName string, params *UpdateKeyspaceConfigParams) error
// UpdateKeyspaceGCManagementType update the `gc_management_type` in keyspace meta config.
// If `gc_management_type` is `global_gc`, it means the current keyspace requires a tidb without 'keyspace-name'
// configured to run a global gc worker to calculate a global gc safe point.
Expand Down Expand Up @@ -1143,19 +1145,28 @@ func (c *client) DeleteOperators(ctx context.Context) error {
WithMethod(http.MethodDelete))
}

// UpdateKeyspaceGCManagementType patches the keyspace config.
func (c *client) UpdateKeyspaceGCManagementType(ctx context.Context, keyspaceName string, keyspaceGCmanagementType *KeyspaceGCManagementTypeConfig) error {
keyspaceConfigPatchJSON, err := json.Marshal(keyspaceGCmanagementType)
func (c *client) updateKeyspaceConfig(ctx context.Context, reqName, keyspaceName string, params any) error {
keyspaceConfigPatchJSON, err := json.Marshal(params)
if err != nil {
return errors.Trace(err)
}
return c.request(ctx, newRequestInfo().
WithName(UpdateKeyspaceGCManagementTypeName).
WithName(reqName).
WithURI(GetUpdateKeyspaceConfigURL(keyspaceName)).
WithMethod(http.MethodPatch).
WithBody(keyspaceConfigPatchJSON))
}

// UpdateKeyspaceConfig patches the keyspace config.
func (c *client) UpdateKeyspaceConfig(ctx context.Context, keyspaceName string, params *UpdateKeyspaceConfigParams) error {
return c.updateKeyspaceConfig(ctx, UpdateKeyspaceConfigName, keyspaceName, params)
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.

Please consider the retry semantics for 409 Conflict here. A keyspace config precondition failure is returned by the server as 409, and this is a deterministic CAS failure, so it should not be retried against other PD members. The current HTTP client noNeedRetry only includes 400/403/404, so this PATCH will continue to be sent to subsequent endpoints after a 409. That can duplicate the same failed semantic request and logs. I suggest either disabling 409 retry for this API, or adding http.StatusConflict to noNeedRetry after confirming no other API relies on retrying 409.

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.

@lhy1024 Modifying noNeedRetry requires further discussion; it's better not to include it in this PR.

}

// UpdateKeyspaceGCManagementType patches the keyspace config.
func (c *client) UpdateKeyspaceGCManagementType(ctx context.Context, keyspaceName string, keyspaceGCmanagementType *KeyspaceGCManagementTypeConfig) error {
return c.updateKeyspaceConfig(ctx, UpdateKeyspaceGCManagementTypeName, keyspaceName, keyspaceGCmanagementType)
}

// GetKeyspaceMetaByName get the given keyspace meta.
func (c *client) GetKeyspaceMetaByName(ctx context.Context, keyspaceName string) (*keyspacepb.KeyspaceMeta, error) {
var (
Expand Down
1 change: 1 addition & 0 deletions client/http/request_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ const (
deletePitrRestoreModeMarkName = "DeletePitrRestoreModeMark"
createOperators = "CreateOperators"
deleteOperators = "DeleteOperators"
UpdateKeyspaceConfigName = "UpdateKeyspaceConfig"
UpdateKeyspaceGCManagementTypeName = "UpdateKeyspaceGCManagementType"
GetKeyspaceMetaByNameName = "GetKeyspaceMetaByName"
GetKeyspaceMetaByIDName = "GetKeyspaceMetaByID"
Expand Down
11 changes: 11 additions & 0 deletions client/http/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,17 @@ type KeyspaceGCManagementTypeConfig struct {
Config KeyspaceGCManagementType `json:"config"`
}

// UpdateKeyspaceConfigParams represents parameters needed to modify target keyspace configs.
// A map of string to string pointer is used to differentiate between json null and "",
// which will both be set to "" if value type is string during marshaling.
type UpdateKeyspaceConfigParams struct {
Config map[string]*string `json:"config"`
// Preconditions specifies prerequisites for updating config, using a JSON-merge-patch-like encoding:
// - key -> nil means the key must be absent.
// - key -> "value" means the key must exist and equal "value".
Preconditions map[string]*string `json:"preconditions,omitempty"`
}

// tempKeyspaceMeta is the keyspace meta struct that returned from the http interface.
type tempKeyspaceMeta struct {
ID uint32 `json:"id"`
Expand Down
61 changes: 61 additions & 0 deletions tests/integrations/client/http_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,67 @@ func (suite *httpClientTestSuite) TestUpdateKeyspaceGCManagementType() {
re.Error(err)
}

func (suite *httpClientTestSuite) TestUpdateKeyspaceConfig() {
re := suite.Require()
client := suite.client
ctx, cancel := context.WithCancel(suite.ctx)
defer cancel()

var keyspaceName string
if kerneltype.IsNextGen() {
keyspaceName = constant.SystemKeyspaceName
} else {
keyspaceName = constant.DefaultKeyspaceName
}

configKey := "http_client_test_update_keyspace_config"
initialValue := "v1"
err := client.UpdateKeyspaceConfig(ctx, keyspaceName, &pd.UpdateKeyspaceConfigParams{
Config: map[string]*string{
configKey: &initialValue,
},
Preconditions: map[string]*string{
configKey: nil,
},
})
re.NoError(err)

keyspaceMetaRes, err := client.GetKeyspaceMetaByName(ctx, keyspaceName)
re.NoError(err)
val, ok := keyspaceMetaRes.Config[configKey]
re.True(ok)
re.Equal(initialValue, val)

wrongExpected := "wrong"
nextValue := "v2"
err = client.UpdateKeyspaceConfig(ctx, keyspaceName, &pd.UpdateKeyspaceConfigParams{
Config: map[string]*string{
configKey: &nextValue,
},
Preconditions: map[string]*string{
configKey: &wrongExpected,
},
})
re.Error(err)
re.Contains(err.Error(), "409 Conflict")
re.Contains(err.Error(), "precondition failed")

err = client.UpdateKeyspaceConfig(ctx, keyspaceName, &pd.UpdateKeyspaceConfigParams{
Config: map[string]*string{
configKey: nil,
},
Preconditions: map[string]*string{
configKey: &initialValue,
},
})
re.NoError(err)

keyspaceMetaRes, err = client.GetKeyspaceMetaByName(ctx, keyspaceName)
re.NoError(err)
_, ok = keyspaceMetaRes.Config[configKey]
re.False(ok)
}

func (suite *httpClientTestSuite) TestGetKeyspaceMetaByID() {
re := suite.Require()
client := suite.client
Expand Down
Loading