[close #138] br return backup ts #156
Conversation
Signed-off-by: haojinming <[email protected]>
Signed-off-by: haojinming <[email protected]>
haojinming
changed the title
Signed-off-by: haojinming <[email protected]>
haojinming
changed the title
br/pkg/backup/client_test.go
Outdated
| @@ -52,7 +54,7 @@ func (r *testBackup) SetUpSuite(c *C) { | |||
| defer httpmock.DeactivateAndReset() | |||
| // Exact URL match | |||
| httpmock.RegisterResponder("GET", `=~^/config`, | |||
| httpmock.NewStringResponder(200, `{"storage":{"api-version":1, "enable-ttl":false}}`)) | |||
| httpmock.NewStringResponder(200, `{"storage":{"api-version":2, "enable-ttl":false}}`)) | |||
There was a problem hiding this comment.
"enable-ttl: true" would be reasonable when api-version is 2.
br/pkg/task/backup_raw.go
Outdated
| @@ -49,6 +51,10 @@ func DefineRawBackupFlags(command *cobra.Command) { | |||
| command.Flags().Bool(flagRemoveSchedulers, false, | |||
| "disable the balance, shuffle and region-merge schedulers in PD to speed up backup.") | |||
|
|
|||
| command.Flags().Duration(flagSafeInterval, utils.DefaultBRSafeInterval, | |||
| "The interval between backup-ts and current tso.") | |||
| command.Flags().Int64(flagGCTTL, utils.DefaultBRGCSafePointTTL, "the TTL (in seconds) that PD holds for BR's GC safepoint") | |||
There was a problem hiding this comment.
Suggest to be a duration argument.
TTL would be expected to be e.g, 30 minutes, or 3 days. It is too big to be in secondes.
| command.Flags().Int64(flagGCTTL, utils.DefaultBRGCSafePointTTL, "the TTL (in seconds) that PD holds for BR's GC safepoint") | |
| command.Flags().Int64(flagGCTTL, utils.DefaultBRGCSafePointTTL, "the TTL of BR's GC safepoint") |
| @@ -49,6 +51,10 @@ func DefineRawBackupFlags(command *cobra.Command) { | |||
| command.Flags().Bool(flagRemoveSchedulers, false, | |||
| "disable the balance, shuffle and region-merge schedulers in PD to speed up backup.") | |||
|
|
|||
| command.Flags().Duration(flagSafeInterval, utils.DefaultBRSafeInterval, | |||
There was a problem hiding this comment.
Suggest to hide safe interval. It's difficult for common users to set this argument.
br/pkg/utils/safe_point.go
Outdated
| // DefaultBRGCSafePointTTL means PD keep safePoint limit at least 5min. | ||
| DefaultBRGCSafePointTTL = 5 * 60 | ||
| // DefaultBRGCSafePointTTL means PD keep safePoint limit at least 72h. | ||
| DefaultBRGCSafePointTTL = 72 * 60 * 60 // 72h |
There was a problem hiding this comment.
Defaults to 72 hours may be too long, and waste disk space if there is no following CDC.
Maybe we can keep 5 minutes, but tell CDC users to set a suitable value.
Signed-off-by: haojinming <[email protected]>
I would like to add integration test for apiv2 and |
Signed-off-by: haojinming <[email protected]>
What problem does this PR solve?
Issue Number: close #138
Problem Description: TBD
What is changed and how does it work?
BR return the backup-ts, which is ts described in issue #138 .
Code changes
Check List for Tests
This PR has been tested by at least one of the following methods:
Side effects
Related changes