Proposal: Merge BR repo into TiDB

Motivation

BR (Backup & Restore) is an independent command-line tool for distributed backup and restoration of the TiDB cluster data.

We would like to put BR code into TiDB repo due to these reasons:

  1. currently BR and TiDB depend each other, It’s not easy to maintain the interdependence golang repo. see details at https://github.com/pingcap/br/issues/345.
  2. BR is a tool only use for TiDB, so put the code together is intuitive, just like other database products.

Steps

Steps below migrates minimal BR codebase to TiDB, which allow BR build and release from the TiDB repository. (Even not the latest version.)

  1. git checkout -b clone-br && git subtree add -D br https://github.com/pingcap/br.git

  2. git checkout -b merge-br (to create a diff branch, so the PR would be easier to review…)

  3. Do necessary merging.

    1. merge go.mod, go.sum
    2. update Makefile,
    3. update the import path for both TiDB and BR.
    4. merge CI scripts.
  4. Create a PR from merge-br targeting clone-br for reviewing.

  5. After reviewing, merge it to master.

Risks

The change in TiDB is add a new directory br and change the import path in executor/brie.go, so the risk is under control.

5 Likes

looks good :smile: Here is a victim. https://github.com/pingcap/tidb/issues/19306

Hi @3pointer ,

Thanks for starting this proposal!

The cycle dependency between pingcap/tidb and pingcap/br is a long trouble story, and this is really a good news to resolve this long time issue.

The proposal looks good generally. I’m concerned which branches we’d like to play this merge actions? Master only? Or also apply on some release branches?

Besides, I’d like to ask for suggestions from TiDB maintainers, e.g., @zhangjinpeng1987 , @bb7133 , @zz-jason , etc.

I think master only.

@3pointer this proposal is neat and clear. I suggest move to a formal proposal on pingcap/tidb repo, see also making a proposal.

@zhangjinpeng1987 who should be involved in this decision making process according to codebase adoption for pingcap/tidb?

Please make sure to update the README of the old repo and archive the pingcap/br repository if possible. Archiving might not be possible as old branches/versions might need to be maintained.

Yes, we are not going to archive the pingcap/br since we still need to maintain this repo. we need to update the READM, thanks for remind.

@dveeden to be clear, we are still maintaining release-4.0, release-5.0 and release-5.1 for BR, after these versions end their life, we can do the archive. @3pointer correct me if I get it wrong.

@3pointer this command doesn’t work.

error: unknown switch `D'
usage: git subtree add   --prefix=<prefix> <commit>
   or: git subtree add   --prefix=<prefix> <repository> <ref>
   or: git subtree merge --prefix=<prefix> <commit>
   or: git subtree pull  --prefix=<prefix> <repository> <ref>
   or: git subtree push  --prefix=<prefix> <repository> <ref>
   or: git subtree split --prefix=<prefix> <commit>

what is the actual step?

I’m concern that whether we have two git history to maintain in one repo, or the history of br end by the merge.

I think perhaps that should be -P or --prefix:thinking:

After we merge BR into TiDB with subtree.

We will get the mixed git log after merged.

If we want to merge a specific commit to BR repo. we can do the following steps

  1. find the commit of add BR(16e1c88195126c22f23f09d28731ec60e1249a42).
  2. git checkout -b one_commit 16e1c88195126c22f23f09d28731ec60e1249a42
  3. write code
  4. git add & git commit
  5. git subtree push --prefix=br one_commit

Thanks!

With an offline discussion with @3pointer I learn git subtree is an one shot action and we may or may not use it to push change set to the original remote repo and generate a PR.

Given that we merge go.mod, Makefile, and so on, we may never use it and git subtree only exists if you use it. See also this page.

Also I found a draft PR. May we create a sibling design doc for it or at least an issue.