added `compact-log-bakcup` to `tikv-ctl` by YuJuncen · Pull Request #17845 · tikv/tikv

YuJuncen · 2024-11-19T10:01:41Z

What is changed and how it works?

Issue Number: Close #17844

What's Changed:
This PR added a new sub-command compact-log-backup to TiKV.
Also this introduced a new type of status server: Lite.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

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

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

None

Signed-off-by: hillium <yujuncen@pingcap.com>

ti-chi-bot · 2024-11-19T10:01:44Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: hillium <yujuncen@pingcap.com>

Signed-off-by: 山岚 <36239017+YuJuncen@users.noreply.github.com>

Tristan1900 · 2024-11-22T13:29:58Z

src/server/status_server/lite.rs

+}
+
+#[derive(Copy, Clone)]
+pub struct LiteService;


quick question, what does Lite mean here?

"light wighted", it truncates most storage-engine related endpoints and only keeps the basic state exporters (mertics, cpu / heap profiling, etc..).

gotcha, if that's case I would recommend to name it StateExportOnlySerivceor something like that, or maybe add a comment to describe what it does?

The comment was added in the module level. See the top of the file.

3pointer · 2024-11-27T05:46:57Z

cmd/tikv-ctl/src/main.rs

+
+impl TemporaryRocks {
+    fn new(cfg: &TikvConfig) -> Result<Self, String> {
+        let tmp = TempDir::new().map_err(|v| format!("failed to create tmp dir: {}", v))?;


How about let the user decide the path of local rocksdb.

I'm concerned that the temp dir may not always be the fast disk in many environments.

In fact we won't write anything to this RocksDB, this RocksDB instance is just used for loading the configuration for creating SstWriter.

👌
then maybe update the comment of db in Execution as well.

Please add some comments to this struct.

3pointer · 2024-11-28T02:49:20Z

cmd/tikv-ctl/src/cmd.rs

+            default_value = "32",
+            help("how many compactions can be executed concurrently.")
+        )]
+        max_compaction_num: u64,


Suggested change

max_compaction_num: u64,

max_concurrent_compactions: u64,

3pointer · 2024-11-28T02:51:59Z

cmd/tikv-ctl/src/cmd.rs

+        )]
+        compression_level: Option<i32>,
+
+        #[structopt(long, help("Don't try to skip already finished compactions."))]


what would happen if re-gen on finished compactions?

It will be regenerated.

So, it's an expected behavior? what does this don't try to skip mean here?

That means skipping checkpoints.

Signed-off-by: hillium <yujuncen@pingcap.com>

3pointer · 2024-11-28T05:22:27Z

rest LGTM

ti-chi-bot · 2024-12-02T07:05:56Z

[LGTM Timeline notifier]

Timeline:

2024-11-28 05:22:49.527955719 +0000 UTC m=+700357.147610235: ☑️ agreed by 3pointer.
2024-12-02 07:05:55.729723198 +0000 UTC m=+1052143.349377713: ☑️ agreed by Leavrth.

overvenus · 2024-12-02T09:39:21Z

cmd/tikv-ctl/src/main.rs

+
+impl TemporaryRocks {
+    fn new(cfg: &TikvConfig) -> Result<Self, String> {
+        let tmp = TempDir::new().map_err(|v| format!("failed to create tmp dir: {}", v))?;


Please add some comments to this struct.

overvenus · 2024-12-02T09:44:29Z

components/compact-log-backup/src/storage.rs

+
+impl From<Migration> for VersionedMigration {
+    fn from(mut mig: Migration) -> Self {
+        mig.set_version(brpb::MigrationVersion::M1);


Out of curiosity, is there a specific reason for the name "M1", as I think "V1" sounds more idiomatic.

Because "V1" has already be used by another enum in the same package, while protocol buffer requires every enum variant (from all enums) has a unique name in a package...

overvenus · 2024-12-02T10:02:47Z

cmd/tikv-ctl/src/cmd.rs

        /// hex end key
        end: String,
    },
+    CompactLogBackup {


I personally think it's better placed in BR, as tikv-ctl is a debugging tool for TiKV's internal state, not part of the backup-restore process.

the deployment is a standalone command in a one-shot pod to scan logs and convert them to sst files. It run offline from normal br process.

ti-chi-bot · 2024-12-02T10:41:32Z

@BornChanger: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

ti-chi-bot · 2024-12-03T02:56:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3pointer, BornChanger, Leavrth, overvenus

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [overvenus]

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

close tikv#17844 Signed-off-by: hillium <yujuncen@pingcap.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com>

* br: batch download and merge download sst before ingest (#19062) close #19086 Add a new PRC method called batch-download to download batch SST. Signed-off-by: RidRisR <79858083+RidRisR@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * fix build Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * make format Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * implement the base library for compacting logs (#17632) close #17631 Added a new crate named `compact-log-backup`. Now it can merge some log files generated by log backup and make them become SSTs. Signed-off-by: hillium <yujuncen@pingcap.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * added `compact-log-bakcup` to `tikv-ctl` (#17845) close #17844 Signed-off-by: hillium <yujuncen@pingcap.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: record `min_input_ts` and `max_input_ts` in Compaction (#18085) close #18084 `min_input_ts` and `max_input_ts` will present in a log files compaction. Signed-off-by: hillium <yu745514916@live.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: fix typo (#18090) ref #15990 Fixed a typo: `Migartion` -> `Migration`. Signed-off-by: hillium <yu745514916@live.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: filter out meta files by migration (#18123) close #18122 Now, `StreamMetaStorage` is able to filter out files by meta edits. Signed-off-by: hillium <yu745514916@live.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: added minimal compactions size (#18235) close #18234 Added `--minimal-compact-size` to `compact-log-backup`. Signed-off-by: hillium <yujuncen@pingcap.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * log backup: fix several issues during compact log backup. (#18298) close #18308 log backup compact: fix several issues during compact a log backup Signed-off-by: 3pointer <luancheng@pingcap.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: correct version assignment in subcompaction metadata (#18389) close #18390 Fixed a bug that caused the time range of compaction generated SSTs are too huge. Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: add new field to track fully compacted data KV files and fix metafile filtering (#18837) close #18843 compact_log_backup: add new field to track fully compacted data KV files and fix metafile filtering Signed-off-by: 3pointer <luancheng@pingcap.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: use max ts among all storage checkpoint ts (#18848) close #18847 Now, `consistency` hook checks the storage checkpoint by the max value among them. Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: fix compact meta edit filter (#18842) close #18843 Merge the same meta edit from different migrations instead of replacing. Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: offload reading meta to diff cpus (#18885) close #18884 This PR spawns read s3 file tasks to remote threads. Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: read meta from checkpoint (#19068) close #19069 This PR makes `compact-log-backup` fills the migration with subcompactions skipped by checkpoint. Signed-off-by: hillium <yu745514916@live.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * fix build Signed-off-by: Juncen Yu <yujuncen@pingcap.com> --------- Signed-off-by: RidRisR <79858083+RidRisR@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> Signed-off-by: hillium <yu745514916@live.com> Signed-off-by: 3pointer <luancheng@pingcap.com> Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com> Signed-off-by: 山岚 <36239017+YuJuncen@users.noreply.github.com> Co-authored-by: ris <79858083+RidRisR@users.noreply.github.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Co-authored-by: 3pointer <luancheng@pingcap.com> Co-authored-by: Jianjun Liao <36503113+Leavrth@users.noreply.github.com>

* br: batch download and merge download sst before ingest (tikv#19062) close tikv#19086 Add a new PRC method called batch-download to download batch SST. Signed-off-by: RidRisR <79858083+RidRisR@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * fix build Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * make format Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * implement the base library for compacting logs (tikv#17632) close tikv#17631 Added a new crate named `compact-log-backup`. Now it can merge some log files generated by log backup and make them become SSTs. Signed-off-by: hillium <yujuncen@pingcap.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * added `compact-log-bakcup` to `tikv-ctl` (tikv#17845) close tikv#17844 Signed-off-by: hillium <yujuncen@pingcap.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: record `min_input_ts` and `max_input_ts` in Compaction (tikv#18085) close tikv#18084 `min_input_ts` and `max_input_ts` will present in a log files compaction. Signed-off-by: hillium <yu745514916@live.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: fix typo (tikv#18090) ref tikv#15990 Fixed a typo: `Migartion` -> `Migration`. Signed-off-by: hillium <yu745514916@live.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: filter out meta files by migration (tikv#18123) close tikv#18122 Now, `StreamMetaStorage` is able to filter out files by meta edits. Signed-off-by: hillium <yu745514916@live.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: added minimal compactions size (tikv#18235) close tikv#18234 Added `--minimal-compact-size` to `compact-log-backup`. Signed-off-by: hillium <yujuncen@pingcap.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * log backup: fix several issues during compact log backup. (tikv#18298) close tikv#18308 log backup compact: fix several issues during compact a log backup Signed-off-by: 3pointer <luancheng@pingcap.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: correct version assignment in subcompaction metadata (tikv#18389) close tikv#18390 Fixed a bug that caused the time range of compaction generated SSTs are too huge. Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: add new field to track fully compacted data KV files and fix metafile filtering (tikv#18837) close tikv#18843 compact_log_backup: add new field to track fully compacted data KV files and fix metafile filtering Signed-off-by: 3pointer <luancheng@pingcap.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: use max ts among all storage checkpoint ts (tikv#18848) close tikv#18847 Now, `consistency` hook checks the storage checkpoint by the max value among them. Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: fix compact meta edit filter (tikv#18842) close tikv#18843 Merge the same meta edit from different migrations instead of replacing. Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: offload reading meta to diff cpus (tikv#18885) close tikv#18884 This PR spawns read s3 file tasks to remote threads. Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * compact_log_backup: read meta from checkpoint (tikv#19068) close tikv#19069 This PR makes `compact-log-backup` fills the migration with subcompactions skipped by checkpoint. Signed-off-by: hillium <yu745514916@live.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * fix build Signed-off-by: Juncen Yu <yujuncen@pingcap.com> --------- Signed-off-by: RidRisR <79858083+RidRisR@users.noreply.github.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> Signed-off-by: hillium <yu745514916@live.com> Signed-off-by: 3pointer <luancheng@pingcap.com> Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com> Signed-off-by: 山岚 <36239017+YuJuncen@users.noreply.github.com> Co-authored-by: ris <79858083+RidRisR@users.noreply.github.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> Co-authored-by: 3pointer <luancheng@pingcap.com> Co-authored-by: Jianjun Liao <36503113+Leavrth@users.noreply.github.com>

added compact-log-bakcup to tikv-ctl

064de89

Signed-off-by: hillium <yujuncen@pingcap.com>

YuJuncen marked this pull request as ready for review November 19, 2024 10:19

YuJuncen added 2 commits November 20, 2024 16:58

make format

20e9de3

Signed-off-by: hillium <yujuncen@pingcap.com>

update migration version

4752e2b

Signed-off-by: hillium <yujuncen@pingcap.com>

ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 22, 2024

Merge branch 'master' into add-compact-log-backup-cmd

601418d

Signed-off-by: 山岚 <36239017+YuJuncen@users.noreply.github.com>

Tristan1900 reviewed Nov 22, 2024

View reviewed changes

3pointer reviewed Nov 27, 2024

View reviewed changes

3pointer reviewed Nov 28, 2024

View reviewed changes

YuJuncen added 3 commits November 28, 2024 11:07

refine tests

63b0472

Signed-off-by: hillium <yujuncen@pingcap.com>

address comments

7b4c51d

Signed-off-by: hillium <yujuncen@pingcap.com>

address comments

780448b

Signed-off-by: hillium <yujuncen@pingcap.com>

3pointer approved these changes Nov 28, 2024

View reviewed changes

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

Leavrth approved these changes Dec 2, 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 2, 2024

Merge branch 'master' into add-compact-log-backup-cmd

90963aa

overvenus reviewed Dec 2, 2024

View reviewed changes

BornChanger approved these changes Dec 2, 2024

View reviewed changes

added some comments for TemporaryRocks

a2b4566

overvenus approved these changes Dec 3, 2024

View reviewed changes

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

ti-chi-bot bot merged commit a6ec27d into tikv:master Dec 3, 2024

ti-chi-bot bot added this to the Pool milestone Dec 3, 2024

YuJuncen mentioned this pull request Dec 5, 2025

tikv-ctl: pick compact-log-backup related stuff to release-8.5 #19169

Merged

9 tasks

Conversation

YuJuncen commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

ti-chi-bot bot commented Nov 19, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

3pointer Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

3pointer commented Nov 28, 2024

Uh oh!

ti-chi-bot bot commented Dec 2, 2024

[LGTM Timeline notifier]

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuJuncen Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overvenus Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Dec 2, 2024

Uh oh!

ti-chi-bot bot commented Dec 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

YuJuncen commented Nov 19, 2024 •

edited

Loading

3pointer Nov 28, 2024 •

edited

Loading

YuJuncen Dec 3, 2024 •

edited

Loading

overvenus Dec 2, 2024 •

edited

Loading