Skip to content

feat(storage): restart and finish part 1, 2 #16

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

Merged
merged 3 commits into from
Feb 25, 2022
Merged

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Feb 20, 2022

The diff history can be found at #15, and the update-storage-tutorial script will copy code from each commit of the storage branch to the main repo.

@skyzh skyzh requested a review from wangrunji0408 February 20, 2022 07:11
@skyzh skyzh force-pushed the skyzh/minor-update branch from b84824e to 8b66679 Compare February 20, 2022 07:12
Copy link
Member

@wangrunji0408 wangrunji0408 left a comment

Choose a reason for hiding this comment

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

LSTM!

For 03-01, now only the last inserted data could be read. Is it a bug or by design?

Click to see the outputs
cargo run -p risinglight-03-01

> create table t(a int);
+---+
| 1 |
+---+

> insert into t values (1), (2);
+---+
| 2 |
+---+

> insert into t values (3), (4);
+---+
| 2 |
+---+

> select * from t;
+---+
| 3 |
| 4 |
+---+

For the test, can we add some features to sqllogictest for testing durability?

@skyzh
Copy link
Member Author

skyzh commented Feb 25, 2022

For 03-01, now only the last inserted data could be read. Is it a bug or by design?

Yes, it's by design. We only support multi-insert after introducing the concept of RowSet.

For the test, can we add some features to sqllogictest for testing durability?

I don't think there's need to do this. Firstly, we don't have manifest. Even if all data are on disk, we won't know there are after restart. Secondly, we don't have mem table, so there's no possibility that data are read from memory.

@skyzh skyzh merged commit 7c66dca into main Feb 25, 2022
@skyzh skyzh deleted the skyzh/minor-update branch February 25, 2022 08:26
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.

2 participants