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

refactor: parallelize open table #1392

Merged

Conversation

shuiyisong
Copy link
Contributor

@shuiyisong shuiyisong commented Apr 17, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

This pr mainly does

  1. replace engine and schema level lock to keylock(using table_ref.to_string()) so that lock is narrowed down to table level
  2. parallelize open table at schema level to speed up initialization

future work

  1. in order to avoid future misuse, I think we should extract tables and mutex into one single function to get table ref and lock at the same return

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

close #1290

@codecov
Copy link

codecov bot commented Apr 17, 2023

Codecov Report

Merging #1392 (7cfcde7) into develop (cc7c313) will decrease coverage by 0.55%.
The diff coverage is 36.19%.

@@             Coverage Diff             @@
##           develop    #1392      +/-   ##
===========================================
- Coverage    85.96%   85.42%   -0.55%     
===========================================
  Files          531      534       +3     
  Lines        79240    79611     +371     
===========================================
- Hits         68120    68006     -114     
- Misses       11120    11605     +485     

@waynexia
Copy link
Member

Have tested S3 backend against this branch, it's faaaaaaster than table_mutex: Mutex<()> now 🚀

src/catalog/src/error.rs Outdated Show resolved Hide resolved
src/catalog/src/remote/manager.rs Outdated Show resolved Hide resolved
src/catalog/src/remote/manager.rs Outdated Show resolved Hide resolved
src/mito/src/engine.rs Outdated Show resolved Hide resolved
shuiyisong and others added 3 commits April 18, 2023 10:26
Co-authored-by: dennis zhuang <killme2008@gmail.com>
Co-authored-by: dennis zhuang <killme2008@gmail.com>
@shuiyisong
Copy link
Contributor Author

shuiyisong commented Apr 18, 2023

@killme2008 PTAL ☺️

Copy link
Contributor

@killme2008 killme2008 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

src/catalog/src/remote/manager.rs Outdated Show resolved Hide resolved
src/catalog/src/remote/manager.rs Outdated Show resolved Hide resolved
@MichaelScofield MichaelScofield merged commit 145f8eb into GreptimeTeam:develop Apr 18, 2023
@shuiyisong shuiyisong deleted the refactor/parallel_open_table branch April 18, 2023 16:07
@killme2008 killme2008 mentioned this pull request May 8, 2023
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* refactor: change open_table to parallel on datanode startup

* chore: try move out register schema table

* chore: change mito engine to key lock

* chore: minor change

* chore: minor change

* chore: update error definition

* chore: remove rwlock on tables

* chore: try parallel register table on schema provider

* chore: add rt log

* chore: add region open rt log

* chore: add actual open region rt log

* chore: add recover rt log

* chore: divide to three part rt log

* chore: remove debug log

* chore: add replay rt log

* chore: update cargo lock

* chore: remove debug log

* chore: revert unused change

* chore: update err msg

Co-authored-by: dennis zhuang <killme2008@gmail.com>

* chore: fix cr issue

Co-authored-by: dennis zhuang <killme2008@gmail.com>

* chore: fix cr issue

* chore: fix cr issue

---------

Co-authored-by: dennis zhuang <killme2008@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parallelize open table while starting up datanode
5 participants