Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breakdown the "dbms/src/Storages/Transaction" module #4646

Closed
3 of 7 tasks
JaySon-Huang opened this issue Apr 13, 2022 · 13 comments · Fixed by #8073 or #8225
Closed
3 of 7 tasks

Breakdown the "dbms/src/Storages/Transaction" module #4646

JaySon-Huang opened this issue Apr 13, 2022 · 13 comments · Fixed by #8073 or #8225
Assignees
Labels
type/code-quality-improvement PR that can improve the code quality type/enhancement The issue or PR belongs to an enhancement.

Comments

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Apr 13, 2022

Enhancement

The codes under "dbms/src/Storages/Transaction" are some codes we adapt TiFlash with TiDB. After years of development, it become kind of crowded and confusing.

Suggest breakdown "dbms/src/Storages/Transaction” into these modules:

  • Some global context for TiFlash management
  • Basic structure from TiDB (TableInfo/DatabaseInfo, etc)
  • Syncing schema with TiDB
  • Decoding data from row to column (Block)
  • Raft layer
    • Region info
    • Handling admin command
    • Handling write command
    • Handling apply snapshot/ingest sst
    • Handling learner read

  1. Suggest creating a dbms/src/TiDB as the module for basic components from TiDB. And
    • Rename TMTContext to TiFlashContext
    • Move TableInfo/DBInfo, etc from TiDB.h to dbms/src/TiDB
    • Move TiFlashContext/ManagedStorages to the dbms/src/TiDB module
    • Add a dbms/src/TiDB/Codec and move the logic of decoding data from row to column there
    • Add a dbms/src/TiDB/Schema and move schema syncing logic there
  2. Rename dbms/src/Transaction to dbms/src/KVStore. And maybe we should further break down the codes for different Raft processes under this module.
@JaySon-Huang JaySon-Huang added the type/enhancement The issue or PR belongs to an enhancement. label Apr 13, 2022
@JaySon-Huang
Copy link
Contributor Author

@hzh0425
Copy link
Contributor

hzh0425 commented Apr 21, 2022

Hi, can you assign this issue to me, I want to try it @JaySon-Huang

@JaySon-Huang
Copy link
Contributor Author

/assign @hzh0425

Yes. And suggest beginning with a PR that does a part of the changes, cause it will change lots of files.

@hzh0425
Copy link
Contributor

hzh0425 commented Apr 21, 2022

/assign @hzh0425

Yes. And suggest beginning with a PR that does a part of the changes, cause it will change lots of files.

Ok, thanks, I will try it

@flowbehappy
Copy link
Contributor

@hzh0425
Good to catch a volunteer!
And instead of submitting a PR, I suggest you can start by making an RFC that includes the basic design and the main parts of the code you would like to modify. We will be very happy to discuss with you and then come up with a great plan.

@JaySon-Huang JaySon-Huang added the type/code-quality-improvement PR that can improve the code quality label May 18, 2022
This was referenced May 19, 2022
ti-chi-bot pushed a commit that referenced this issue May 30, 2022
@detongz
Copy link

detongz commented Aug 21, 2022

Hi, I think I can help with this refactor.

I am a user of tidb, sync data to another database with ticdc is slow which costs 1s at least, so I came up with an idea to change tiflash source code and impliment our own data storage.

Will this refactor support 5.4.0? We are using version 5.4.0 in production.

@JaySon-Huang
Copy link
Contributor Author

Hi @mischaZhang, it would be great if you could help with this refactor!

Will this refactor support 5.4.0? We are using version 5.4.0 in production.

Sorry that we only pick bug fixes to the later patch version of 5.4.x, so this refactor won't be backported to release-5.4 on our repo.

@detongz
Copy link

detongz commented Aug 22, 2022

Hi @mischaZhang, it would be great if you could help with this refactor!

Will this refactor support 5.4.0? We are using version 5.4.0 in production.

Sorry that we only pick bug fixes to the later patch version of 5.4.x, so this refactor won't be backported to release-5.4 on our repo.

I may be help with this refactor on September, please assign me some not so complicated tasks to begin with.

And I really would like to hear your thoughts on my "use tiflash as data sync" idea, thank you for replying :D

@JaySon-Huang
Copy link
Contributor Author

I may be help with this refactor on September, please assign me some not so complicated tasks to begin with.

Maybe you can begin with the task "Move TableInfo/DBInfo, etc from TiDB.h to dbms/src/TiDB". There are some closed PRs that you can take a look at: #4976, #4915.

@JaySon-Huang
Copy link
Contributor Author

And I really would like to hear your thoughts on my "use tiflash as data sync" idea

Maybe you can describe more about the design of how do you reuse the TiFlash code for syncing data to another database?

Based on my knowledge, handling the table schema change in TiFlash is not as easy as using TiCDC

@detongz
Copy link

detongz commented Aug 23, 2022

And I really would like to hear your thoughts on my "use tiflash as data sync" idea

Maybe you can describe more about the design of how do you reuse the TiFlash code for syncing data to another database?

Based on my knowledge, handling the table schema change in TiFlash is not as easy as using TiCDC

I watched the DDL introducion live, good news is that we don't really change schemas in months.

I want to capture row change events, these data are captured in doLearnerRead. To make data syncronized faster than TiCDC, maybe sending some my-system-related rpc calls before Tiflash flush data into delta store to sync my own data.

@JaySon-Huang
Copy link
Contributor Author

/assign @mischaZhang

@JaySon-Huang JaySon-Huang mentioned this issue Sep 11, 2023
12 tasks
@CalvinNeo CalvinNeo assigned CalvinNeo and detongz and unassigned detongz Sep 11, 2023
ti-chi-bot bot pushed a commit that referenced this issue Sep 12, 2023
@CalvinNeo
Copy link
Member

It is not fully finished

@CalvinNeo CalvinNeo reopened this Sep 12, 2023
@JaySon-Huang JaySon-Huang mentioned this issue Sep 18, 2023
12 tasks
ti-chi-bot bot pushed a commit that referenced this issue Sep 18, 2023
@CalvinNeo CalvinNeo mentioned this issue Oct 20, 2023
12 tasks
ti-chi-bot bot pushed a commit that referenced this issue Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/code-quality-improvement PR that can improve the code quality type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants