pd: RawKV GC SafePoint API #908
Conversation
|
|
AmoebaProtozoa
changed the title
AmoebaProtozoa
force-pushed
the
PD_GC_API
branch
from
abeb7db to
534559f
Compare
AmoebaProtozoa
changed the title
|
@nolouch @MyonKeminta @TonsnakeLin PTAL, thanks ~ |
proto/pdpb.proto
Outdated
| @@ -67,12 +67,6 @@ service PD { | |||
|
|
|||
| rpc ScatterRegion(ScatterRegionRequest) returns (ScatterRegionResponse) {} | |||
|
|
|||
| rpc GetGCSafePoint(GetGCSafePointRequest) returns (GetGCSafePointResponse) {} | |||
There was a problem hiding this comment.
Are there compatibility issues with existing interfaces being removed? It is recommended to test it, otherwise it needs to be reserved as a Deprecated interface first.
There was a problem hiding this comment.
Ok, So how about keeping the old GC apis in pdpb (until we decide to phase it out), adding only the new ones to a separate gcpb?
AmoebaProtozoa
force-pushed
the
PD_GC_API
branch
from
28403d4 to
da8c2a1
Compare
Signed-off-by: AmoebaProtozoa <[email protected]>
Signed-off-by: AmoebaProtozoa <[email protected]>
Signed-off-by: AmoebaProtozoa <[email protected]>
Signed-off-by: AmoebaProtozoa <[email protected]>
Signed-off-by: AmoebaProtozoa <[email protected]>
Signed-off-by: AmoebaProtozoa <[email protected]>
Signed-off-by: AmoebaProtozoa <[email protected]>
AmoebaProtozoa
force-pushed
the
PD_GC_API
branch
from
da8c2a1 to
79ebfa7
Compare
proto/gcpb.proto
Outdated
| } | ||
|
|
||
| message GetAllServiceGroupsRequest { | ||
| pdpb.RequestHeader header = 1; |
There was a problem hiding this comment.
It would better to be independent to PD's RequestHeader & ResponseHeader:
- To make GC service possible to be totally independent to PD (e.g, a standalone micro service in Cloud).
- Define error type of PD & GC service together seems not reasonable.
There was a problem hiding this comment.
Should the same separation be done on PD side?
Say, move gc methods outside grpc_service.go? And use separate type GcServer to provide gc functionalities instead of GrpcServer? https://github.com/tikv/pd/blob/6ab2e4448c331de6dbedb61dbec5113e1a2a4122/server/grpc_service.go#L72
What about client? Is a separate gc client needed along side PD client?
There was a problem hiding this comment.
I think yes, and we can try. Unless it is too expensive to do so. @nolouch How do you think ?
There was a problem hiding this comment.
yes, PD client may include gc client first?
for the header, may define a more general header? it can be used in PD(replace the old), also can use to gc service.
Signed-off-by: AmoebaProtozoa <[email protected]>
|
LGTM~ |
As described by tikv/rfcs#90
Added 5 new RPC services as well as their corresponding messages related to RawKV GC