Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

add uploader compress writer #566

Merged
merged 24 commits into from
Nov 16, 2020
Merged

add uploader compress writer #566

merged 24 commits into from
Nov 16, 2020

Conversation

lichunzhu
Copy link
Contributor

What problem does this PR solve?

pingcap/dumpling#7

What is changed and how it works?

Add uploader compress writer

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has exported function/method change
  • Has exported variable/fields change

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • None

@lichunzhu
Copy link
Contributor Author

/rebuild

@lichunzhu
Copy link
Contributor Author

/run-all-tests

3 similar comments
@lichunzhu
Copy link
Contributor Author

/run-all-tests

@3pointer
Copy link
Collaborator

/run-all-tests

@lichunzhu
Copy link
Contributor Author

/run-all-tests

kennytm
kennytm reviewed Oct 27, 2020
View reviewed changes
pkg/storage/writer.go Outdated Show resolved Hide resolved
kennytm
kennytm reviewed Oct 27, 2020
View reviewed changes
Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM

pkg/storage/writer.go Outdated Show resolved Hide resolved
pkg/storage/writer.go Outdated Show resolved Hide resolved
@lichunzhu
Copy link
Contributor Author

/run-all-tests

@lichunzhu
Copy link
Contributor Author

/run-all-tests

@lichunzhu
Copy link
Contributor Author

/rebuild

kennytm
kennytm reviewed Nov 2, 2020
View reviewed changes
Copy link
Collaborator

@kennytm kennytm left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I wonder why do we want to support zlib 🤷 ("other compression algorithms" mostly means lz4 and zstd)

@kennytm
Copy link
Collaborator

kennytm commented Nov 2, 2020

@lichunzhu i mean we can keep simpleCompressBuffer just not add the zlib support 👀

@lichunzhu
Copy link
Contributor Author

OK

@lichunzhu
Copy link
Contributor Author

/rebuild

@lichunzhu
Copy link
Contributor Author

/rebuild

@lichunzhu
Copy link
Contributor Author

/run-all-tests

1 similar comment
@lichunzhu
Copy link
Contributor Author

/run-all-tests

@kennytm
Copy link
Collaborator

kennytm commented Nov 4, 2020

/run-integration-test

https://internal.pingcap.net/idc-jenkins/blue/organizations/jenkins/br_ghpr_unit_and_integration_test/detail/br_ghpr_unit_and_integration_test/3609/pipeline

Hmm can't read the log at all.

Error: failed to validate checksum: [BR:Restore:ErrRestoreChecksumMismatch]restore checksum mismatch

@lichunzhu
Copy link
Contributor Author

/run-integration-test

@lichunzhu
Copy link
Contributor Author

/rebuild

1 similar comment
@lichunzhu
Copy link
Contributor Author

/rebuild

@lichunzhu
Copy link
Contributor Author

/run-all-tests

@kennytm
Copy link
Collaborator

kennytm commented Nov 10, 2020

(retrigger CLA)

@kennytm kennytm closed this Nov 10, 2020
@kennytm kennytm reopened this Nov 10, 2020
@lichunzhu
Copy link
Contributor Author

/rebuild

@lichunzhu
Copy link
Contributor Author

/run-all-tests

3pointer
3pointer approved these changes Nov 16, 2020
View reviewed changes
Copy link
Collaborator

@3pointer 3pointer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@3pointer 3pointer merged commit 0b4e2a9 into pingcap:master Nov 16, 2020
@lichunzhu lichunzhu deleted the addUploaderCompressWriter branch November 16, 2020 03:22
@ti-srebot ti-srebot mentioned this pull request Nov 16, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #595

lichunzhu pushed a commit that referenced this pull request Nov 17, 2020
@ti-srebot
add uploader compress writer (#566) (#595)
9acd5a9 
* add uploader compress writer
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kennytm kennytm kennytm left review comments

@3pointer 3pointer 3pointer approved these changes

@overvenus overvenus Awaiting requested review from overvenus

Assignees
No one assigned
Labels
needs-cherry-pick-release-4.0 status/LGT1 LGTM1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lichunzhu @3pointer @kennytm @ti-srebot