RFC: reorganize packages layout for this repository

[bb7133]

(Comments & suggestions are appreciated!)

Motivation

As you might notice, the codebase of this repository is organized in a ‘flattened’ way: there are 40+ level-1 directories/packages, which, in my option, has the following cons:

(1) It looks tedious. (2) Similar to (1), the hierarchical structure is not clear enough.

So, here I would like to propose the reorganization of the packages.

The structure

As shown in the figure below, I would like to make the following changes:

• Move the package of Data Platform tools(br and dumpling) to tools directory.
• Move tidb-server package to cmd directory.
• Add pkg directory, which contains the packages for "TiDB kernel"(most of the level-1 directories today).

tidb
├── LICENSES
├── cmd
│   ├── tidb-server
|   └── ... 
├── build
├── docs
├── hooks
├── pkg
|   ├── autoid_service
|   ├── bindinfo
│   ├── config
│   ├── ddl
│   ├── distsql
│   ├── domain
│   ├── errno
│   ├── executor
│   ├── extension
│   ├── errno
│   ├── infoschema
│   ├── keyspace
│   ├── kv
│   ├── lock
│   ├── meta
│   ├── metrics
│   ├── owner
│   ├── meta
│   ├── parser
│   ├── planner
│   ├── plugin
│   ├── resourcemanager
│   ├── server
│   ├── session
│   ├── sessionctx
│   ├── sessiontxn
│   ├── statistics
│   ├── store
│   ├── table
│   ├── tablecodec
│   ├── telemetry
│   ├── testkit
│   ├── tidb-binlog
│   ├── ttl
│   ├── types
│   └── util
│   ├── ttl
|   └── util
|       ├── structure
|       └── ...
├── tests
├── tools
│   ├── br
│   ├── check
│   └── dumpling
└── ...

[lance6716]

We are really looking forward to it! And I have some considerations:

1. the tidb-server in the root is a directory that only contains the main package? can we use a cmd directory which will also include tools' main packages under sub-directory?
2. tidb-lightning is put under br/pkg/lightning, we want to change its place in PR as well. And for tidb-lightning, part of its functionalities is used by DDL, maybe it's also a good chance to move some lightning packages into kernel.
3. I can imagine a lots of git conflict for opening PRs, how will we reduce the impact? For example, provide a reproducing script for moving the package?

[bb7133]

1. the tidb-server in the root is a directory that only contains the main package?

Yes, I think so.

can we use a cmd directory which will also include tools' main packages under sub-directory?

Well, we have cmd directory already, are you talking about combing pingcap/tidb/br/cmd, and also moving tidb-server under cmd? I think it should be fine.

I can imagine a lots of git conflict for opening PRs, how will we reduce the impact? For example, provide a reproducing script for moving the package?

I don't think we can provide any script(don't know how to do it). I just think that if this is accepted, we will make it on some low-traffic time like weekend. Please let me know if you have any better idea.

---

Update: will make a try.

[hawkingrei]

This is a related issue.

https://github.com/pingcap/tidb/issues/27762

[hawkingrei]

@bb7133 Some components are huge and have many codes. But They such as executor put all codes in the same folder. Will they improve packages in this plan?

[lance6716]

This is my suggestion

├── cmd
│   ├── br
│   ├── dumpling
│   ├── tests
│   │   ├── benchdb
...
│   ├── tidb-lightning-ctl
│   ├── tidb-lightning
│   └── tidb-server

And do you have any experience can help with moving packages? @amyangfei

[bb7133]

@bb7133 Some components are huge and have many codes. But They such as executor put all codes in the same folder. Will they improve packages in this plan?

Nope, I think the current proposal requires a lot of changes already. For further changes, we could make them one by one.

[dveeden]

Are we going to update other repos?

e.g. https://github.com/pingcap/dumpling/blob/master/README.md and https://github.com/pingcap/tidb-lightning/blob/master/README.md ? I think we should only do that for those that are not archived.

this would also needs updates in pingcap/docs and pingcap/docs-cn

[hawkingrei]

Is there any progress?

[wjhuang2016]

There are some util packages in the BR directory, storage, for example. we'd better move them out of the BR directory.

[wuhuizuo]

My suggestion:

• Avoid package names like util, or common...

And what's the differences between pkg/server and tidb-server, it makes me confused.

[hawkingrei]

My suggestion:

* Avoid package names like `util`, or `common`...

And what's the differences between pkg/server and tidb-server, it makes me confused.

util is so sommon package. many open source project has util package.

https://github.com/prometheus/prometheus/tree/main/util https://github.com/kubernetes/kubernetes/tree/master/pkg/util https://github.com/cockroachdb/cockroach/tree/master/pkg/util https://github.com/argoproj/argo-workflows/tree/master/util

Additionally, utilizing code from the util package as much as possible can enhance development efficiency and improve the quality of code validation across multiple scenarios due to code reuse.

server is the server component. in general, it is tidb entry. This is a common naming convention.

for example

https://github.com/kubernetes/kubernetes/tree/master/pkg/kubelet/server

[bb7133]

This is my suggestion

├── cmd
│   ├── br
│   ├── dumpling
│   ├── tests
│   │   ├── benchdb
...
│   ├── tidb-lightning-ctl
│   ├── tidb-lightning
│   └── tidb-server

And do you have any experience can help with moving packages? @amyangfei

I'm trying to provide a script so that when we make the change, the on-going PRs can be updated easily. However, if we make some changes for lightning, my concern is that the script would be complicated.

Considering the impact of changing lightning directory will be limited to a small scope, I would like to split the PR into several:

1. Move most of the package to pkg and move br and dumpling to tools or cmd.
2. Update the structure tools or cmd, the change would be limited in 1 package so it does not introduce big impact to the other PRs.
3. ...

For the choice of cmd / tools to put br and dumpling in, I don't have a strong opinion for now.

[wuhuizuo]

My suggestion:

* Avoid package names like `util`, or `common`...

And what's the differences between pkg/server and tidb-server, it makes me confused.

util is so sommon package. many open source project has util package.

https://github.com/prometheus/prometheus/tree/main/util https://github.com/kubernetes/kubernetes/tree/master/pkg/util https://github.com/cockroachdb/cockroach/tree/master/pkg/util https://github.com/argoproj/argo-workflows/tree/master/util

Additionally, utilizing code from the util package as much as possible can enhance development efficiency and improve the quality of code validation across multiple scenarios due to code reuse.

server is the server component. in general, it is tidb entry. This is a common naming convention.

for example

https://github.com/kubernetes/kubernetes/tree/master/pkg/kubelet/server

About utils or common, many projects have done this way doesn't mean it's appropriate. We shouldn’t just look for one-sided examples. it's pkg/server is not pkg/tidb/server, but it is tiny.

[bb7133]

This is my suggestion

├── cmd
│   ├── br
│   ├── dumpling
│   ├── tests
│   │   ├── benchdb
...
│   ├── tidb-lightning-ctl
│   ├── tidb-lightning
│   └── tidb-server

And do you have any experience can help with moving packages? @amyangfei

@lance6716 I got your point finally: there is tidb/br/cmd already, you suggested merging it into tidb/cmd. IMHO it's a good idea!

For the rest part of tidb/br, should we move it to tidb/tools/br?

[YangKeao]

My suggestion:

* Avoid package names like `util`, or `common`...

And what's the differences between pkg/server and tidb-server, it makes me confused.

@wuhuizuo Do you have any suggestion on where we should place packages like /util/mathutil and /util/fastrand?

[bb7133]

Are we going to update other repos?

e.g. https://github.com/pingcap/dumpling/blob/master/README.md and https://github.com/pingcap/tidb-lightning/blob/master/README.md ? I think we should only do that for those that are not archived.

this would also needs updates in pingcap/docs and pingcap/docs-cn

@dveeden , Yes, of course!

[wuhuizuo]

My suggestion:

* Avoid package names like `util`, or `common`...

And what's the differences between pkg/server and tidb-server, it makes me confused.

@wuhuizuo Do you have any suggestion on where we should place packages like /util/mathutil and /util/fastrand?

/util/fastrand => /pkg/fastrand ? I think it's OK.