Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

PlainTable support #358

Merged
merged 7 commits into from
Jun 23, 2021
Merged

PlainTable support #358

merged 7 commits into from
Jun 23, 2021

Conversation

critical27
Copy link
Contributor

To support rocksdb PlainTable and nebula work in memory

  1. Dir of rocksdb wal and nebula wal should be configurable
  2. Backup rocksdb periodically, and try to recover from latest backup
  3. Fix the 2.0 prefix bloom filter

@@ -114,6 +125,8 @@ RocksEngine::RocksEngine(GraphSpaceID spaceId,
db_.reset(db);
partsNum_ = allParts().size();
LOG(INFO) << "open rocksdb on " << path;

backup();
Copy link
Contributor

Choose a reason for hiding this comment

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

A little confused, seems like a run for nothing, could give some comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Long story, I can't remember very clear.

  1. If we don't backup here, we write some data, and storage is stopped before we do a valid backup.
  2. Mock machine reboot, so rocksdb data in memory is lost. (rocksdb wal is persisted in disk, so we could recover from it in theory)
  3. Restart the storage, rocks db will be failed to open, log is While creating a new Db, wal_dir contains existing log file: ....

I believe the problem is that if we backup here, it is not nothing here. Something like CURRENT and MANIFEST will be backuped. In that way, rocksdb will think the wal is legal.

It involves some rocksdb code, so complicated. Then I try this way and it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. It's a very complicated story.

@CLAassistant
Copy link

CLAassistant commented Mar 26, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

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

Good job. Well done.

@critical27 critical27 force-pushed the plain branch 3 times, most recently from 28d9d53 to 0375814 Compare June 19, 2021 09:48
Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@panda-sheep panda-sheep left a comment

Choose a reason for hiding this comment

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

Woohoo,Great job!
I learned a lot from it.👍

@bright-starry-sky bright-starry-sky merged commit d6b61b8 into vesoft-inc:master Jun 23, 2021
@critical27 critical27 deleted the plain branch June 23, 2021 07:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants