add db-upgrade V3 #3417

cangfengzhs · 2021-12-06T10:11:34Z

What type of PR is this?

bug
feature
enhancement

What does this PR do?

Which issue(s)/PR(s) this PR relates to?

Special notes for your reviewer, ex. impact of this fix, etc:

Additional context:

Checklist：

Documentation affected （Please add the label if documentation needs to be modified.)
Incompatible （If it is incompatible, please describe it and add corresponding label.）
Need to cherry-pick （If need to cherry-pick to some branches, please label the destination version(s).)
Performance impacted: Consumes more CPU/Memory

Release notes：

Please confirm whether to reflect in release notes and how to describe:

codecov-commenter · 2021-12-20T03:07:58Z

Codecov Report

Merging #3417 (6399d5a) into master (99f1f7a) will increase coverage by 0.02%.
The diff coverage is 95.06%.

@@            Coverage Diff             @@
##           master    #3417      +/-   ##
==========================================
+ Coverage   85.19%   85.22%   +0.02%     
==========================================
  Files        1304     1307       +3     
  Lines      121308   122545    +1237     
==========================================
+ Hits       103352   104433    +1081     
- Misses      17956    18112     +156

Impacted Files	Coverage Δ
src/clients/storage/StorageClient.h	`100.00% <ø> (ø)`
src/clients/storage/StorageClientBase-inl.h	`74.61% <ø> (+1.93%)`	⬆️
src/graph/validator/MutateValidator.h	`100.00% <ø> (ø)`
src/storage/mutate/AddEdgesProcessor.h	`100.00% <ø> (ø)`
src/storage/mutate/AddVerticesProcessor.h	`100.00% <ø> (ø)`
src/clients/meta/MetaClient.cpp	`74.97% <50.00%> (+0.43%)`	⬆️
src/storage/mutate/AddEdgesProcessor.cpp	`59.86% <78.57%> (+3.20%)`	⬆️
src/clients/storage/StorageClient.cpp	`78.68% <100.00%> (+0.24%)`	⬆️
src/graph/executor/mutate/InsertExecutor.cpp	`100.00% <100.00%> (ø)`
src/graph/planner/plan/Mutate.h	`100.00% <100.00%> (ø)`
... and 105 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6ddeb2...6399d5a. Read the comment docs.

critical27

Generally LGTM, write a storage version mark to each rocksdb.
We will check meta's version when we call waitForMetadReady, after meta is upgraded to V3, you could check it later, the PR is coming.
BTW, better rename upgrade_version.

critical27 · 2021-12-23T02:17:49Z

src/tools/db-upgrade/NebulaKeyUtilsV3.cpp

+  key.resize(key.size() - sizeof(TagID));
+  return key;
+}
+std::string NebulaKeyUtilsV3::dataVersionKey() { return "\xFF\xFF\xFF\xFF"; }


You could move it to NebulaKeyUtils

wenhaocs · 2021-12-23T02:24:40Z

src/tools/db-upgrade/DbUpgrader.cpp

+              "",
+              "When the value is 1:2, upgrade the data from 1.x to 2.0 GA. "
+              "When the value is 2RC:2, upgrade the data from 2.0 RC to 2.0 GA."
+              "When the value is 2:3, upgrade the data from ");


darionyaphet · 2021-12-23T02:49:07Z

src/tools/db-upgrade/DbUpgrader.cpp

+  }
+  auto code = readEngine_->ingest(ingest_sst_file_, true);
+  if (code != ::nebula::cpp2::ErrorCode::SUCCEEDED) {
+    LOG(FATAL) << "Faild upgrade 2:3 when ingest sst file:" << static_cast<int>(code);


LOG(FATAL) will cause crash ?

Yes, and if ingest failed, upgrader can do nothing except crash.

panda-sheep · 2021-12-23T02:58:05Z

src/tools/db-upgrade/DbUpgrader.cpp

@@ -83,7 +86,7 @@ Status UpgraderSpace::initSpace(const std::string& sId) {

  // Use readonly rocksdb
  readEngine_.reset(new nebula::kvstore::RocksEngine(
-      spaceId_, spaceVidLen_, srcPath_, "", nullptr, nullptr, true));
+      spaceId_, spaceVidLen_, srcPath_, "", nullptr, nullptr, false));


why change?

This rocksdb is originally a read-only

Because upgrade to 3.0 just need append new data by ingest sst file and write a dataVersionKey to identity the data encode version. So there is no need to write a new rocksdb, just upgrade in place

Confused about readEngine_.reset a writeable one

read means source and write means destnation

src/tools/db-upgrade/DbUpgrader.cpp

cangfengzhs added 3 commits December 6, 2021 18:09

add db-upgrade V3

aabff08

Merge remote-tracking branch 'vesoft/master' into upgrade

3b62a1a

use ingest

1f51f3b

cangfengzhs added the ready-for-testing PR: ready for the CI test label Dec 15, 2021

cangfengzhs requested review from critical27, a team, panda-sheep and pengweisong December 15, 2021 08:57

cangfengzhs marked this pull request as ready for review December 15, 2021 08:57

Merge branch 'master' into upgrade

62f7d98

critical27 reviewed Dec 21, 2021

View reviewed changes

cangfengzhs added 3 commits December 21, 2021 17:39

modify upgrade args

84a3581

write data version key

3a0412b

Merge branch 'upgrade' of github.com:cangfengzhs/nebula into upgrade

b8c118a

critical27 reviewed Dec 23, 2021

View reviewed changes

critical27 previously approved these changes Dec 23, 2021

View reviewed changes

wenhaocs reviewed Dec 23, 2021

View reviewed changes

wenhaocs previously approved these changes Dec 23, 2021

View reviewed changes

Sophie-Xie added the LGTM2 label Dec 23, 2021

darionyaphet reviewed Dec 23, 2021

View reviewed changes

darionyaphet previously approved these changes Dec 23, 2021

View reviewed changes

address wenhao's comment

6399d5a

cangfengzhs dismissed stale reviews from darionyaphet, wenhaocs, and critical27 via 6399d5a December 23, 2021 02:54

panda-sheep reviewed Dec 23, 2021

View reviewed changes

cangfengzhs requested a review from critical27 December 23, 2021 04:40

SuperYoko reviewed Dec 23, 2021

View reviewed changes

src/tools/db-upgrade/DbUpgrader.cpp Outdated Show resolved Hide resolved

SuperYoko reviewed Dec 23, 2021

View reviewed changes

src/tools/db-upgrade/DbUpgrader.cpp Show resolved Hide resolved

cangfengzhs added 2 commits December 23, 2021 15:12

address some comment

7145f6b

address some comment

7b92dab

wenhaocs previously approved these changes Dec 23, 2021

View reviewed changes

Merge branch 'master' into upgrade

b4c80ad

yixinglu removed the LGTM2 label Dec 25, 2021

cangfengzhs added 3 commits December 27, 2021 10:37

Merge remote-tracking branch 'vesoft/master' into upgrade

06db765

format

fdbf0c4

Merge remote-tracking branch 'origin/upgrade' into upgrade

cd9b0e5

cangfengzhs dismissed wenhaocs’s stale review via cd9b0e5 December 27, 2021 03:07

cangfengzhs requested review from wenhaocs, darionyaphet, SuperYoko and panda-sheep December 27, 2021 04:37

SuperYoko approved these changes Dec 27, 2021

View reviewed changes

Sophie-Xie added the ready for review label Dec 27, 2021

Merge branch 'master' into upgrade

9c2ff71

panda-sheep approved these changes Dec 28, 2021

View reviewed changes

Merge branch 'master' into upgrade

19782be

critical27 merged commit a89e5b3 into vesoft-inc:master Dec 28, 2021

jamieliu1023 mentioned this pull request Jan 1, 2022

Weekly Report 2021-12-31 vesoft-inc/nebula-community#83

Closed

Sophie-Xie added the doc affected PR: improvements or additions to documentation label Jan 4, 2022

cooper-lzy requested a review from randomJoe211 January 5, 2022 03:01

Sophie-Xie mentioned this pull request Jan 29, 2022

upgrade for v2.x(v2.0.0-GA or later) to v3.0.0 #3399

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add db-upgrade V3 #3417

add db-upgrade V3 #3417

cangfengzhs commented Dec 6, 2021 •

edited by Sophie-Xie

Loading

codecov-commenter commented Dec 20, 2021 •

edited

Loading

critical27 left a comment

critical27 Dec 23, 2021

wenhaocs Dec 23, 2021

cangfengzhs Dec 23, 2021

darionyaphet Dec 23, 2021

cangfengzhs Dec 23, 2021

panda-sheep Dec 23, 2021

panda-sheep Dec 23, 2021

cangfengzhs Dec 23, 2021

panda-sheep Dec 23, 2021

wenhaocs Dec 23, 2021

cangfengzhs Dec 23, 2021

add db-upgrade V3 #3417

add db-upgrade V3 #3417

Conversation

cangfengzhs commented Dec 6, 2021 • edited by Sophie-Xie Loading

What type of PR is this?

What does this PR do?

Which issue(s)/PR(s) this PR relates to?

Special notes for your reviewer, ex. impact of this fix, etc:

Additional context:

Checklist：

Release notes：

codecov-commenter commented Dec 20, 2021 • edited Loading

Codecov Report

critical27 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cangfengzhs commented Dec 6, 2021 •

edited by Sophie-Xie

Loading

codecov-commenter commented Dec 20, 2021 •

edited

Loading