Skip to content

perf: Optimize table catalog cache #170

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

Closed
wants to merge 1 commit into from

Conversation

crwen
Copy link
Member

@crwen crwen commented Mar 17, 2024

What problem does this PR solve?

Every time run a sql will create a KipTransaction and they all have their all table catalog cache. But table catalog cache may be not changed all the the time. So I move table catalog cache to KipStorage, and make KipTransaction share this cache.

Issue link: #162

What is changed and how it works?

Code changes

  • Has Rust code change
  • Has CI related scripts change

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Note for reviewer

@KKould
Copy link
Member

KKould commented Mar 17, 2024

This is a good idea. In issue: #162, it is a costly thing to get the table every time, but the reason why I have not done this is that I don’t After knowing this, will the table affect the correctness of the transaction? This may need to be considered when a transaction exists in DDL, DQL, or DML, and table_cache can only be actually modified when it is submitted.

I know that SQLite may have such a function, but in my impression SQLite only supports a single write transaction at the same time, while FnckSQL can allow multiple write transactions at the same time.

I don't think what pr can currently see is able to get the results right, it can be a very complicated thing

Tips: I deliberately set the table_cache size in KipTransaction to be smaller because I think that only a few tables are often involved in a single transaction.

@KKould KKould self-requested a review March 17, 2024 18:00
@KKould KKould added the enhancement New feature or request label Mar 17, 2024
@KKould
Copy link
Member

KKould commented Mar 18, 2024

I found that DDL should only have one transaction at the same time, are you interested in implementing this? because it can implement table_cache, the issue is here: #171,

@KKould KKould added perf and removed enhancement New feature or request labels Mar 18, 2024
@KKould
Copy link
Member

KKould commented Mar 21, 2024

I found a similar concept: mysql mdl(metadata lock), we can refer to this to implement it in the Database layer instead of the Server layer

@KKould
Copy link
Member

KKould commented Mar 25, 2024

merged on #177

@KKould KKould closed this Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants