add s3 api to adapt cdc log (#456)#463
Conversation
* add s3 api to adapt cdc log Co-authored-by: kennytm <kennytm@gmail.com>
ca4cb12 to
ef9bead
Compare
|
The function signature of |
|
thanks but i don't see how that is relevant to this PR (S3 API) |
|
Before force pushing, |
| // function; the second argument is the size in byte of the file determined | ||
| // by path. | ||
| func (l *LocalStorage) WalkDir(ctx context.Context, fn func(string, int64) error) error { | ||
| func (l *LocalStorage) WalkDir(ctx context.Context, dir string, listCount int64, fn func(string, int64) error) error { |
There was a problem hiding this comment.
if the dir parameter means a specific sub dir to walk, the local implements should only walk the sub dir instead of ther whole base dir?
There was a problem hiding this comment.
I'll fix it in next PR. because this is a cherry-pick PR
| // function; the second argument is the size in byte of the file determined | ||
| // by path. | ||
| func (rs *S3Storage) WalkDir(ctx context.Context, fn func(string, int64) error) error { | ||
| func (rs *S3Storage) WalkDir(ctx context.Context, dir string, listCount int64, fn func(string, int64) error) error { |
There was a problem hiding this comment.
As the listCount parameter (or maybe also the dir param) is only used for s3 implement as a contextal value, I think It's better to put this kind of control param into a abstract ctx interface{} field. Maybe there are other similar contextal paramers need for gcs for example?
There was a problem hiding this comment.
I'll move these parameters in ctx after this PR merged, because this PR is a cherry-pick PR.
| // UploadPart upload part of file data to storage | ||
| UploadPart(ctx context.Context, data []byte) error | ||
| // CompleteUpload make the upload data to a complete file | ||
| CompleteUpload(ctx context.Context) error |
There was a problem hiding this comment.
IMO, to name this as Close to be consistent with other io like interface is
a little better, anyway this is ok to me.
|
/run-integration-tests |
|
|
||
| // NeedAutoID checks whether the table needs backing up with an autoid. | ||
| func NeedAutoID(tblInfo *model.TableInfo) bool { | ||
| hasRowID := !tblInfo.PKIsHandle && !tblInfo.IsCommonHandle |
|
LGTM |
cherry-pick #456 to release-4.0
What problem does this PR solve?
part of pingcap/ticdc#826, to unify all s3 related api in the same repo. we decide to put new s3 api which ticdc needed into br.
What is changed and how it works?
move ticdc s3 api in br
Check List
Tests
Release Note