tso/local: remove local tso completely #8864

okJiang · 2024-12-02T10:09:24Z

What problem does this PR solve?

Issue Number: Close #8802

What is changed and how does it work?

Check List

Tests

Unit test
Integration test

Release note

None.

Signed-off-by: okJiang <[email protected]>

lhy1024

LGTM, but ci failed

pkg/mcs/tso/server/server.go

rleungx · 2024-12-03T05:55:23Z

pkg/tso/allocator_manager.go

@@ -217,17 +214,6 @@ func (am *AllocatorManager) getGroupIDStr() string {
 	return strconv.FormatUint(uint64(am.kgID), 10)
 }

-// GetTimestampPath returns the timestamp path in etcd.
-func (am *AllocatorManager) GetTimestampPath() string {


GetTimestampPath is only used to print log when created allocator manager now. I think

log.Info("created allocator manager", zap.Uint32("keyspace-group-id", group.ID))

is enough, do not have to know the TimestampPath.

The comment is important and we also use it in the test.

Did you say this comment?

// GetTimestampPath returns the timestamp path in etcd, which is: // 1. for the default keyspace group: // a. timestamp in /pd/{cluster_id}/timestamp // b. lta/{dc-location}/timestamp in /pd/{cluster_id}/lta/{dc-location}/timestamp // 1. for the non-default keyspace groups: // a. {group}/gts/timestamp in /ms/{cluster_id}/tso/{group}/gta/timestamp // b. {group}/lts/{dc-location}/timestamp in /ms/{cluster_id}/tso/{group}/lta/{dc-location}/timestamp GetTimestampPath() string

It is outdated. lta/{dc-location}/timestamp in /pd/{cluster_id}/lta/{dc-location}/timestamp and {group}/lts/{dc-location}/timestamp in /ms/{cluster_id}/tso/{group}/lta/{dc-location}/timestamp are removed. We can see the left in

pd/pkg/utils/keypath/key_path.go

Lines 356 to 366 in 3cfd66f

// KeyspaceGroupGlobalTSPath constructs the timestampOracle path prefix for Global TSO, which is:

// 1. for the default keyspace group:

// "" in /pd/{cluster_id}/timestamp

// 2. for the non-default keyspace groups:

// {group}/gta in /ms/{cluster_id}/tso/{group}/gta/timestamp

func KeyspaceGroupGlobalTSPath(groupID uint32) string {

if groupID == constant.DefaultKeyspaceGroupID {

return ""

}

return path.Join(fmt.Sprintf("%05d", groupID), globalTSOAllocatorEtcdPrefix)

}

Got it. BTW, I found we still have some comments mentioning KeyspaceGroupLocalTSPath, please also remove it.

Signed-off-by: okJiang <[email protected]>

codecov · 2024-12-03T06:18:44Z

Codecov Report

Attention: Patch coverage is 75.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 76.23%. Comparing base (fbfcdb8) to head (983b58a).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8864      +/-   ##
==========================================
+ Coverage   76.22%   76.23%   +0.01%     
==========================================
  Files         461      461              
  Lines       70448    70395      -53     
==========================================
- Hits        53696    53664      -32     
+ Misses      13399    13383      -16     
+ Partials     3353     3348       -5

Flag	Coverage Δ
unittests	`76.23% <75.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Signed-off-by: okJiang <[email protected]>

okJiang · 2024-12-03T08:24:30Z

/retest

okJiang · 2024-12-09T06:39:18Z

/cc @lhy1024 @JmPotato

JmPotato · 2024-12-09T07:12:46Z

client/client.go

+// Deprecated: Currently, regardless of the
 // parameters passed in, this method will default to returning the global TSO.


Suggested change

// Deprecated: Currently, regardless of the

// parameters passed in, this method will default to returning the global TSO.

// Deprecated: the Local TSO feature has been deprecated. Regardless of the parameters passed, the behavior of this interface will be equivalent to `GetTSAsync/GetTS`. If you want to use a separately deployed TSO service, please refer to the deployment of the TSO microservice.

JmPotato · 2024-12-09T07:18:47Z

client/client.go

@@ -582,7 +582,7 @@ func (c *client) GetTS(ctx context.Context) (physical int64, logical int64, err

 // GetLocalTS implements the TSOClient interface.
 //
-// Deprecated: Local TSO will be completely removed in the future. Currently, regardless of the
+// Deprecated: Currently, regardless of the


Signed-off-by: okJiang <[email protected]>

okJiang · 2024-12-10T02:54:01Z

/retest

Signed-off-by: okJiang <[email protected]>

okJiang · 2024-12-11T06:10:04Z

/retest

ti-chi-bot · 2024-12-11T06:29:16Z

[LGTM Timeline notifier]

Timeline:

2024-12-03 08:41:21.165612388 +0000 UTC m=+1144268.785266898: ☑️ agreed by rleungx.
2024-12-11 06:29:15.530131654 +0000 UTC m=+419945.618934192: ☑️ agreed by JmPotato.

ti-chi-bot · 2024-12-11T07:04:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JmPotato, niubell, rleungx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [JmPotato,rleungx]
~~pkg/mcs/tso/server/OWNERS~~ [niubell]
~~server/config/OWNERS~~ [niubell]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…o-svr-4

remove local tso

444b0db

Signed-off-by: okJiang <[email protected]>

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the dco. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 2, 2024

Merge branch 'master' of github.com:tikv/pd into local-tso-svr-4

14a8c86

Signed-off-by: okJiang <[email protected]>

lhy1024 reviewed Dec 2, 2024

View reviewed changes

rleungx reviewed Dec 3, 2024

View reviewed changes

fix ci

eedb168

Signed-off-by: okJiang <[email protected]>

fix typo

f8796e3

Signed-off-by: okJiang <[email protected]>

rleungx approved these changes Dec 3, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 3, 2024

ti-chi-bot bot requested review from JmPotato and lhy1024 December 9, 2024 06:39

JmPotato reviewed Dec 9, 2024

View reviewed changes

okJiang added 2 commits December 10, 2024 10:38

fix comment

fe0f17b

Signed-off-by: okJiang <[email protected]>

Merge branch 'master' of github.com:tikv/pd into local-tso-svr-4

dce1261

modify comment

cf0d46a

Signed-off-by: okJiang <[email protected]>

JmPotato approved these changes Dec 11, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Dec 11, 2024

Merge branch 'master' into local-tso-svr-4

182c18c

niubell approved these changes Dec 11, 2024

View reviewed changes

ti-chi-bot bot added the approved label Dec 11, 2024

okJiang added 2 commits December 11, 2024 15:50

Merge branch 'master' of github.com:tikv/pd into local-tso-svr-4

159c991

Merge branch 'local-tso-svr-4' of github.com:okJiang/pd into local-ts…

983b58a

…o-svr-4

ti-chi-bot bot merged commit 5d62787 into tikv:master Dec 11, 2024
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tso/local: remove local tso completely #8864

tso/local: remove local tso completely #8864

okJiang commented Dec 2, 2024

lhy1024 left a comment

rleungx Dec 3, 2024

okJiang Dec 3, 2024 •

edited

Loading

rleungx Dec 3, 2024 •

edited

Loading

okJiang Dec 3, 2024

rleungx Dec 3, 2024

codecov bot commented Dec 3, 2024 •

edited

Loading

okJiang commented Dec 3, 2024

okJiang commented Dec 9, 2024

JmPotato Dec 9, 2024

JmPotato Dec 9, 2024

okJiang commented Dec 10, 2024

okJiang commented Dec 11, 2024

ti-chi-bot bot commented Dec 11, 2024

ti-chi-bot bot commented Dec 11, 2024

	// KeyspaceGroupGlobalTSPath constructs the timestampOracle path prefix for Global TSO, which is:
	// 1. for the default keyspace group:
	// "" in /pd/{cluster_id}/timestamp
	// 2. for the non-default keyspace groups:
	// {group}/gta in /ms/{cluster_id}/tso/{group}/gta/timestamp
	func KeyspaceGroupGlobalTSPath(groupID uint32) string {
	if groupID == constant.DefaultKeyspaceGroupID {
	return ""
	}
	return path.Join(fmt.Sprintf("%05d", groupID), globalTSOAllocatorEtcdPrefix)
	}

		// Deprecated: Currently, regardless of the
		// parameters passed in, this method will default to returning the global TSO.

	// Deprecated: Currently, regardless of the
	// parameters passed in, this method will default to returning the global TSO.
	// Deprecated: the Local TSO feature has been deprecated. Regardless of the parameters passed, the behavior of this interface will be equivalent to `GetTSAsync/GetTS`. If you want to use a separately deployed TSO service, please refer to the deployment of the TSO microservice.

tso/local: remove local tso completely #8864

tso/local: remove local tso completely #8864

Conversation

okJiang commented Dec 2, 2024

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

lhy1024 left a comment

Choose a reason for hiding this comment

rleungx Dec 3, 2024

Choose a reason for hiding this comment

okJiang Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

rleungx Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

okJiang Dec 3, 2024

Choose a reason for hiding this comment

rleungx Dec 3, 2024

Choose a reason for hiding this comment

codecov bot commented Dec 3, 2024 • edited Loading

Codecov Report

okJiang commented Dec 3, 2024

okJiang commented Dec 9, 2024

JmPotato Dec 9, 2024

Choose a reason for hiding this comment

JmPotato Dec 9, 2024

Choose a reason for hiding this comment

okJiang commented Dec 10, 2024

okJiang commented Dec 11, 2024

ti-chi-bot bot commented Dec 11, 2024

[LGTM Timeline notifier]

ti-chi-bot bot commented Dec 11, 2024

okJiang Dec 3, 2024 •

edited

Loading

rleungx Dec 3, 2024 •

edited

Loading

codecov bot commented Dec 3, 2024 •

edited

Loading