RFC: reorganize packages layout for this repository #41241

bb7133 · 2023-02-09T12:37:25Z

(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
└── ...

The text was updated successfully, but these errors were encountered:

lance6716 · 2023-02-10T03:02:18Z

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

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?
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.
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 · 2023-02-10T06:21:17Z

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 · 2023-02-10T17:50:15Z

This is a related issue.

#27762

hawkingrei · 2023-02-11T13:11:30Z

@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 · 2023-02-13T02:28:43Z

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 · 2023-02-15T05:30:12Z

@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 · 2023-02-15T07:39:16Z

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 · 2023-04-07T08:48:18Z

Is there any progress?

wjhuang2016 · 2023-08-01T08:45:12Z

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

wuhuizuo · 2023-09-12T09:34:00Z

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 · 2023-09-12T12:04:10Z

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 · 2023-09-13T13:51:21Z

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:

Move most of the package to pkg and move br and dumpling to tools or cmd.
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.
...

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

wuhuizuo · 2023-09-15T03:04:02Z

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 · 2023-09-15T04:15:38Z

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 · 2023-09-15T08:31:40Z

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 · 2023-09-15T08:36:58Z

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 · 2023-09-26T10:16:20Z

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.

… queue (#971) ref pingcap/tidb#41241

ref #41241

close #41241

bb7133 added the type/enhancement The issue or PR belongs to an enhancement. label Feb 9, 2023

bb7133 pinned this issue Feb 9, 2023

fzzf678 unpinned this issue Feb 10, 2023

bb7133 pinned this issue Feb 10, 2023

ran-huang mentioned this issue Feb 17, 2023

Add lightning log message explanation doc pingcap/docs#11930

Merged

15 tasks

AndreMouche unpinned this issue Apr 14, 2023

hawkingrei mentioned this issue May 15, 2023

timer: add user interfaces for timer framework #43837

Merged

12 tasks

AndreMouche pinned this issue May 16, 2023

bb7133 mentioned this issue Sep 20, 2023

*: move most of the packages to 'pkg' #47123

Merged

4 tasks

hawkingrei mentioned this issue Sep 21, 2023

*: move util into pkg/util #47151

Closed

13 tasks

lcwangchao mentioned this issue Sep 21, 2023

*: move util into pkg/util #47158

Closed

13 tasks

bb7133 mentioned this issue Oct 8, 2023

(CI TEST): move most packages to pkg/ #47457

Closed

4 tasks

wuhuizuo mentioned this issue Oct 11, 2023

config(tidb): prioritize the PRs with Priority/P* labels in merging queue ti-community-infra/configs#971

Merged

bb7133 changed the title ~~RFC: reorganize packages for this repository~~ RFC: reorganize packages layout for this repository Oct 12, 2023

ti-chi-bot bot pushed a commit to ti-community-infra/configs that referenced this issue Oct 13, 2023

config(tidb): prioritize the PRs with Priority/P* labels in merging…

68a79c3

… queue (#971) ref pingcap/tidb#41241

ti-chi-bot bot pushed a commit that referenced this issue Oct 13, 2023

*: move most of the packages to 'pkg' (#47123)

318e82b

ref #41241

you06 mentioned this issue Nov 13, 2023

parser: fix some deadlinks #48539

Merged

13 tasks

ti-chi-bot bot closed this as completed in #48539 Nov 13, 2023

ti-chi-bot bot pushed a commit that referenced this issue Nov 13, 2023

parser: fix some deadlinks (#48539)

6d00d10

close #41241

hawkingrei reopened this Nov 14, 2023

tangenta mentioned this issue Nov 29, 2023

docs: update parser quickstart #48993

Merged

13 tasks

tangenta mentioned this issue Dec 7, 2023

README.md: update link pingcap/parser#1428

Merged

BornChanger linked a pull request Dec 20, 2023 that will close this issue

br: refactor phase 1 cleanup files #49626

Open

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: reorganize packages layout for this repository #41241

RFC: reorganize packages layout for this repository #41241

bb7133 commented Feb 9, 2023 •

edited

Loading

lance6716 commented Feb 10, 2023 •

edited

Loading

bb7133 commented Feb 10, 2023 •

edited

Loading

hawkingrei commented Feb 10, 2023

hawkingrei commented Feb 11, 2023 •

edited

Loading

lance6716 commented Feb 13, 2023

bb7133 commented Feb 15, 2023

dveeden commented Feb 15, 2023

hawkingrei commented Apr 7, 2023

wjhuang2016 commented Aug 1, 2023

wuhuizuo commented Sep 12, 2023 •

edited

Loading

hawkingrei commented Sep 12, 2023

bb7133 commented Sep 13, 2023

wuhuizuo commented Sep 15, 2023 •

edited

Loading

bb7133 commented Sep 15, 2023

YangKeao commented Sep 15, 2023

bb7133 commented Sep 15, 2023

wuhuizuo commented Sep 26, 2023 •

edited

Loading

RFC: reorganize packages layout for this repository #41241

RFC: reorganize packages layout for this repository #41241

Comments

bb7133 commented Feb 9, 2023 • edited Loading

Motivation

The structure

lance6716 commented Feb 10, 2023 • edited Loading

bb7133 commented Feb 10, 2023 • edited Loading

hawkingrei commented Feb 10, 2023

hawkingrei commented Feb 11, 2023 • edited Loading

lance6716 commented Feb 13, 2023

bb7133 commented Feb 15, 2023

dveeden commented Feb 15, 2023

hawkingrei commented Apr 7, 2023

wjhuang2016 commented Aug 1, 2023

wuhuizuo commented Sep 12, 2023 • edited Loading

hawkingrei commented Sep 12, 2023

bb7133 commented Sep 13, 2023

wuhuizuo commented Sep 15, 2023 • edited Loading

bb7133 commented Sep 15, 2023

YangKeao commented Sep 15, 2023

bb7133 commented Sep 15, 2023

wuhuizuo commented Sep 26, 2023 • edited Loading

bb7133 commented Feb 9, 2023 •

edited

Loading

lance6716 commented Feb 10, 2023 •

edited

Loading

bb7133 commented Feb 10, 2023 •

edited

Loading

hawkingrei commented Feb 11, 2023 •

edited

Loading

wuhuizuo commented Sep 12, 2023 •

edited

Loading

wuhuizuo commented Sep 15, 2023 •

edited

Loading

wuhuizuo commented Sep 26, 2023 •

edited

Loading