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

Index of edges and vertices in storage layer #1360

Merged
merged 10 commits into from
Jan 7, 2020
Merged

Index of edges and vertices in storage layer #1360

merged 10 commits into from
Jan 7, 2020

Conversation

bright-starry-sky
Copy link
Contributor

Support for edges and vertices insert、delete and update

@nebula-community-bot
Copy link
Member

Unit testing failed.

@bright-starry-sky
Copy link
Contributor Author

Jenkins go

@nebula-community-bot
Copy link
Member

Unit testing passed.

src/common/base/NebulaKeyUtils.cpp Outdated Show resolved Hide resolved
src/common/base/NebulaKeyUtils.cpp Outdated Show resolved Hide resolved
src/common/base/NebulaKeyUtils.cpp Outdated Show resolved Hide resolved
src/interface/storage.thrift Outdated Show resolved Hide resolved
src/interface/storage.thrift Outdated Show resolved Hide resolved
src/interface/storage.thrift Outdated Show resolved Hide resolved
src/interface/storage.thrift Outdated Show resolved Hide resolved
src/interface/storage.thrift Outdated Show resolved Hide resolved
@bright-starry-sky
Copy link
Contributor Author

Addressed all @dangleptr comments since 2019-12-15 10:15:00:
1: Using kIndex instead of kData.
2: Removed 'kIndexPrefix' from index key.
3: Removed 'IndexItem' from all thrift requests.
4: Avoid the use of asyncAtomicOp if no index exists.
5: Avoid the double copy for NebulaKeyUtils::vertexIndexKey and NebulaKeyUtils::edgeIndexKey.

@nebula-community-bot
Copy link
Member

Unit testing passed.

1 similar comment
@nebula-community-bot
Copy link
Member

Unit testing passed.

@bright-starry-sky
Copy link
Contributor Author

Resolve the conflict

@nebula-community-bot
Copy link
Member

Unit testing passed.

std::for_each(req.parts.begin(), req.parts.end(), [&](auto& partEdges){
auto partId = partEdges.first;
const auto &edges = partEdges.second;
auto atomic = [&]() -> std::string {
Copy link
Contributor

@critical27 critical27 Dec 18, 2019

Choose a reason for hiding this comment

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

Maybe you should capture by value here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't quite follow you. your means [&] ?

@@ -88,6 +88,59 @@ std::string NebulaKeyUtils::kvKey(PartitionID partId, const folly::StringPiece&
return key;
}

// static
void NebulaKeyUtils::indexRaw(const IndexValues &values, std::string& raw) {
std::vector<int32_t> colsLen;
Copy link
Contributor

Choose a reason for hiding this comment

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

colsLen init with size first better

edge.key.ranking, edge.key.dst, version);
data.emplace_back(std::move(key), std::move(edge.get_props()));
if (indexes_.empty()) {
std::for_each(req.parts.begin(), req.parts.end(), [&](auto& partEdges){
Copy link
Contributor

Choose a reason for hiding this comment

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

no need capture all local variables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

your means [&] ?

@nebula-community-bot
Copy link
Member

Unit testing passed.

@nebula-community-bot
Copy link
Member

Unit testing passed.

* kv(part1_src1_edgeType1_rank1_dst1 , v4)
*
* Ultimately, kv(part1_src1_edgeType1_rank1_dst1 , v4) . It's just what I need.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

*/

static std::string encodeInt64(int64_t v) {
v ^= folly::to<int64_t>(1) << 63;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about define a constexpr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A good idea, Do you mean for method encodeInt64 or constant folly::to<int64_t>(1) << 63?

@nebula-community-bot
Copy link
Member

Unit testing passed.

@bright-starry-sky
Copy link
Contributor Author

Performance optimization for index write .
The performance report please refer to #1580

@nebula-community-bot
Copy link
Member

Unit testing passed.

src/storage/mutate/AddEdgesProcessor.cpp Show resolved Hide resolved
@@ -16,3 +16,5 @@ DEFINE_int32(waiting_catch_up_interval_in_secs, 30,
DEFINE_int32(waiting_new_leader_retry_times, 30, "retry times when waiting for catching up data");
DEFINE_int32(waiting_new_leader_interval_in_secs, 5,
"interval between two requests for catching up state");
DEFINE_bool(ignore_index_check_pre_insert, false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Well done.

}
raw.append(col.second.data(), col.second.size());
}
for (auto len : colsLen) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why append column size at the end of encoded-value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used for variable-length column type , for example string .
if an index row likes string('abc') + string('abc') , when the where expression is col1 == 'abca', the current row should be skip.

spaceId_,
u.first);
const auto &cols = index.get_cols();
auto values = collectIndexValues(std::move(reader).get(), cols);
Copy link
Contributor

Choose a reason for hiding this comment

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

why std::move(reader) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why std::move(reader) ?

Good point! It got me thinking about performance optimization. I've solved the performance problem for delete|update vertices or edges.

spaceId_,
u.first);
const auto &oCols = index.get_cols();
auto oValues = collectIndexValues(std::move(oReader).get(), oCols);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@bright-starry-sky
Copy link
Contributor Author

1,Performance optimization. 2,Resolve the conflict. 3,Improve test cases

@nebula-community-bot
Copy link
Member

Unit testing passed.

Copy link
Contributor

@dangleptr dangleptr left a comment

Choose a reason for hiding this comment

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

Well done. The PR looks good to me now.

@nebula-community-bot
Copy link
Member

Unit testing passed.

@dutor dutor merged commit fc1760e into vesoft-inc:master Jan 7, 2020
@jude-zhu
Copy link
Contributor

jude-zhu commented Feb 13, 2020

close #458 & #467

@bright-starry-sky bright-starry-sky deleted the online_index branch February 27, 2020 06:12
tong-hao pushed a commit to tong-hao/nebula that referenced this pull request Jun 1, 2021
* online index

* Address all comments since 2019-12-15 10:15:00

* Resolve the conflict

* Addressed all @dangleptr comments since 2019-12-26 23:00:00

* Addressed dangleptr's comment since 2019-12-27 21:00:00

* Addressed dangleptr's comment since 2019-12-30 17:45:00

* Improved processing logic forAddEdges and AddVertices

* Performance optimization for index write

* 1,Performance optimization. 2,Resolve the conflict. 3,Improve test cases

1: Using kIndex instead of kData.
2: Removed 'kIndexPrefix' from index key.
3: Removed 'IndexItem' from all thrift requests.
4: Avoid the use of asyncAtomicOp if no index exists.
5: Avoid the double copy for NebulaKeyUtils::vertexIndexKey and NebulaKeyUtils::edgeIndexKey.

Co-authored-by: yaphet <darion.wang@vesoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants