add verteKey

[cangfengzhs]

check vertex key in fetch

What type of PR is this?

• bug
• feature
• enhancement

What does this PR do?

part of support vertex without tag

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

#3123
Close #2725

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

There are two remaining issues

1. Fetch returns EMPTY. At first I wanted to change it to return NULL, but I found that graph would change the execution flow based on the result returned by storage. If storage returns EMPTY, graph will think that the corresponding property has not been obtained, and will make a request again in the subsequent execution process; if storage returns NULL, graph will think that the value of the property has been obtained, which is NULL.

If it is acceptable to return to EMPTY, there is no need to make changes. Otherwise I need some help from someone who is familiar with graph service.

2. The issue of consistency between vertex and tag. For example, suppose there is a vertex and two tags, v1(t1,t2).Update v1(t1) and Delete v1 are executed concurrently in any order, the final result should be that v1 does not exist (of course, t1, t2 also does not exist), but in the current scheme, the data may be v1(t1) after the execution is completed.

In the original design, we considered using locks to maintain consistency, but because the implementation of locks is more complicated, and other people have proposed a simpler solution, we did not adopt the lock solution. But at present, there is still a situation of breaking consistency without using a lock. This problem will be fixed in the future.

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]

Codecov Report

Merging #3328 (21987fb) into master (ab73b4c) will increase coverage by 0.02%.
The diff coverage is 96.15%.

@@            Coverage Diff             @@
##           master    #3328      +/-   ##
==========================================
+ Coverage   85.23%   85.25%   +0.02%     
==========================================
  Files        1277     1277              
  Lines      119088   119115      +27     
==========================================
+ Hits       101502   101555      +53     
+ Misses      17586    17560      -26     

Impacted Files	Coverage Δ
src/common/utils/NebulaKeyUtils.h	80.68% <ø> (ø)
src/common/utils/Types.h	100.00% <ø> (ø)
src/graph/optimizer/rule/CollapseProjectRule.cpp	98.24% <ø> (ø)
...timizer/rule/OptimizeEdgeIndexScanByFilterRule.cpp	90.54% <ø> (ø)
...ptimizer/rule/OptimizeTagIndexScanByFilterRule.cpp	90.27% <ø> (ø)
...h/optimizer/rule/PushLimitDownGetNeighborsRule.cpp	93.54% <ø> (ø)
...timizer/rule/PushStepLimitDownGetNeighborsRule.cpp	93.75% <ø> (ø)
...imizer/rule/PushStepSampleDownGetNeighborsRule.cpp	93.93% <ø> (ø)
src/graph/planner/plan/Query.h	97.61% <ø> (+0.16%)	⬆️
src/graph/util/FTIndexUtils.cpp	4.12% <ø> (ø)
... and 43 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 dd06a53...21987fb. Read the comment docs.

[Shylock-Hg]

Will storing tag/vertex in separate places decrease performance of getting properties of vertex?

[cangfengzhs]

There will be a decline, but it is negligible for the following reasons:

1. Only when the tag to be read does not exist, will the vertex be judged based on the vertexKey. The probability of this situation is very low.
2. Even if the vertexKey is read, it is only a get() request, and the overhead is very small.
3. Someone previously proposed a solution to put vertexKey and tagKey together, and use the method of prefix for query. But the performance of rocksdb's seek() and iterate() is very poor, which may be dozens of times that of get(). This approach will bring performance degradation instead. Of course this is just a theoretical analysis, I will do a simple experiment if necessary.

[Shylock-Hg]

ok

[critical27]

LGTM

[critical27]

1. I don't quite remember the design, it seems we need to write vertexKey when delete tag
2. Where do we delete the vertexKey? Not in this PR? Just delete them when we delete vertex

[cangfengzhs]

1. I don't quite remember the design, it seems we need to write vertexKey when delete tag
2. Where do we delete the vertexKey? Not in this PR? Just delete them when we delete vertex

1. We don't need write vertexKey when delete tag, because this will not generate a separate（without vertexKey） tag key.
2. delete vertex is already in this PR, just in DeleteVerticesProcessor.

[critical27]

1. I don't quite remember the design, it seems we need to write vertexKey when delete tag
2. Where do we delete the vertexKey? Not in this PR? Just delete them when we delete vertex

1. We don't need write vertexKey when delete tag, because this will not generate a separate（without vertexKey） tag key.
2. delete vertex is already in this PR, just in DeleteVerticesProcessor.

1. I don't quite remember the design, it seems we need to write vertexKey when delete tag
2. Where do we delete the vertexKey? Not in this PR? Just delete them when we delete vertex

1. We don't need write vertexKey when delete tag, because this will not generate a separate（without vertexKey） tag key.
2. delete vertex is already in this PR, just in DeleteVerticesProcessor.

I see it, my bad.

The reason I suggest to write vertexKey when delete tag is that user only want to delete a tag, in this case, user believe the vertex do exists. In other words, we can regard delete tag is special type of update.

[cangfengzhs]

I see it, my bad.

The reason I suggest to write vertexKey when delete tag is that user only want to delete a tag, in this case, user believe the vertex do exists. In other words, we can regard delete tag is special type of update.

But if user delete a tag whose vertex is not exist, this will insert a vertexKey. This is very abnormal.

[yixinglu]

We shouldn't return a result with all properties empty value when the vertex doesn't exist. The empty response is what we expect.

[cangfengzhs]

Because we add a vertexKey, Delete Tag will not delete vertex at same time. Vertex is still existed

[czpmango]

Do we plan to support fetch tags on hash("Tim Duncan") later?
This seems like a natural user demand scenario bcz TagKey and VertexKey are abstracted out.

[Shylock-Hg]

LGTM

[Shylock-Hg]

Close #2725