fix(core): edge cache not clear when update or delete associated vertex #1780

sunlujing · 2022-03-12T13:13:34Z

fix the bug when vertex updated, but the edge cache associated with that vertex was not updated. Here we simply clear all edge cache when any vertex changes.
add two test case for the fixing

1. fix the bug when vertex updated, but the edge cache associated with that vertex was not updated. Here we simply clear all edge cache when any vertex changes.

1.add two test case for the fixing

1. fix the bug when vertex updated, but the edge cache associated with that vertex was not updated. Here we simply clear all edge cache when any vertex changes.

CLAassistant · 2022-03-12T13:13:39Z

All committers have signed the CLA.

codecov · 2022-03-14T02:55:06Z

Codecov Report

Merging #1780 (fdbe63a) into master (5bb860f) will increase coverage by 3.93%.
The diff coverage is 50.00%.

@@             Coverage Diff              @@
##             master    #1780      +/-   ##
============================================
+ Coverage     66.86%   70.79%   +3.93%     
+ Complexity      972      970       -2     
============================================
  Files           443      443              
  Lines         37748    37749       +1     
  Branches       5383     5384       +1     
============================================
+ Hits          25240    26726    +1486     
+ Misses         9784     8302    -1482     
+ Partials       2724     2721       -3

Impacted Files	Coverage Δ
...ugegraph/backend/cache/CachedGraphTransaction.java	`85.48% <50.00%> (+4.40%)`	⬆️
...va/com/baidu/hugegraph/util/collection/IntSet.java	`73.61% <0.00%> (-1.28%)`	⬇️
...m/baidu/hugegraph/backend/tx/GraphTransaction.java	`80.16% <0.00%> (+0.10%)`	⬆️
...du/hugegraph/backend/tx/GraphIndexTransaction.java	`83.72% <0.00%> (+0.21%)`	⬆️
...du/hugegraph/traversal/optimize/TraversalUtil.java	`62.60% <0.00%> (+0.21%)`	⬆️
.../baidu/hugegraph/backend/query/ConditionQuery.java	`85.14% <0.00%> (+0.24%)`	⬆️
...du/hugegraph/schema/builder/IndexLabelBuilder.java	`88.69% <0.00%> (+0.28%)`	⬆️
.../com/baidu/hugegraph/auth/StandardAuthManager.java	`92.81% <0.00%> (+0.32%)`	⬆️
...idu/hugegraph/schema/builder/EdgeLabelBuilder.java	`83.62% <0.00%> (+0.34%)`	⬆️
...a/com/baidu/hugegraph/backend/query/Condition.java	`77.97% <0.00%> (+0.36%)`	⬆️
... and 109 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 5bb860f...fdbe63a. Read the comment docs.

imbajin

Thanks for your contribution, a little comments for it.

For code style problems, if you use IDEA as your IDE, you can directly import our code style configuration file.

imbajin · 2022-03-14T02:52:50Z

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/cache/CachedGraphTransactionTest.java

+
+
+    @Test
+    public void testClearEdgeCacheWhenDeleteVertex(){


testClearEdgeCacheXx --> testEdgeCacheClearXx or testEdgeCacheEmptyXx seems better, same as another test method

imbajin · 2022-03-14T02:57:37Z

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/cache/CachedGraphTransaction.java

+            // Update edge cache if any vertex or edge change
+            // for vertex change, the edge associated with that vertex should also be updated
+            // here we just clear all the edge cache , before we use a more precise strategy


some typo:

we could use /* xx */ and let them looks like a paragraph

vertex change --> vertex changed

here xx, before xxx --> Before xx. now we just xx

imbajin · 2022-03-14T02:59:43Z

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/cache/CachedGraphTransactionTest.java

+import com.baidu.hugegraph.structure.HugeEdge;
+import com.baidu.hugegraph.structure.HugeVertexProperty;


we could move the import lines after 35, recommend using our code-style config to avoid these changes~

imbajin · 2022-03-14T03:00:57Z

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/cache/CachedGraphTransactionTest.java

+                .idStrategy(IdStrategy.CUSTOMIZE_NUMBER)
+                .properties("name").nullableKeys("name")
+                .checkExist(false)
+                .create();


we could keep original format. so as other lines (align lines)

javeme

Thanks for your contribution! The code logic looks good, can we adjust the code style?
You can refer to the code style guide: https://github.com/hugegraph/hugegraph-doc/wiki/HugeGraph%E4%BB%A3%E7%A0%81%E9%A3%8E%E6%A0%BC%E6%8C%87%E5%8D%97

javeme · 2022-03-14T03:44:30Z

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/cache/CachedGraphTransactionTest.java

}
+

can remove useless line

javeme · 2022-03-14T03:44:42Z

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/cache/CachedGraphTransactionTest.java

+
+

can remove useless line

javeme · 2022-03-14T03:48:27Z

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/cache/CachedGraphTransactionTest.java

+    }
+
+    @Test
+    public void testClearEdgeCacheWhenUpdateVertex(){


Thanks for contributing test cases

format code using hugegraph code style

sunlujing · 2022-03-14T04:35:46Z

have reformated code style

imbajin · 2022-03-14T06:21:33Z

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/cache/CachedGraphTransaction.java

+            /*
+             * Update edge cache if any vertex or edge changed
+             * For vertex change, the edges linked with should also be updated
+             * Before we use a more precise strategy,now we just clear all the edge cache


enhance to--> Before we find a more precise strategy, just clear all the edge cache now (expect a space after ,)

PS: For convenience, u can edit the file directly on github page 😄

enhance the comment

javeme · 2022-03-14T07:04:57Z

hugegraph-test/src/main/java/com/baidu/hugegraph/unit/cache/CachedGraphTransactionTest.java

+
+

one blank line is ok

remove one blank line

github-actions · 2022-03-14T07:18:32Z

CLA Assistant Lite bot Good! All Contributors have signed the CLA.

sunlujing · 2022-03-14T07:22:22Z

I have read the CLA Document and I hereby sign the CLA

sunlujing · 2022-03-14T07:22:38Z

recheck

sunlujing added 3 commits March 12, 2022 21:00

Update CachedGraphTransaction.java

a75e4c5

1. fix the bug when vertex updated, but the edge cache associated with that vertex was not updated. Here we simply clear all edge cache when any vertex changes.

Update CachedGraphTransactionTest.java

376700c

1.add two test case for the fixing

Update CachedGraphTransaction.java

3ef5d38

1. fix the bug when vertex updated, but the edge cache associated with that vertex was not updated. Here we simply clear all edge cache when any vertex changes.

imbajin reviewed Mar 14, 2022

View reviewed changes

javeme reviewed Mar 14, 2022

View reviewed changes

sunlujing added 2 commits March 14, 2022 12:30

Update CachedGraphTransaction.java

9f21a98

format code using hugegraph code style

Update CachedGraphTransactionTest.java

652670f

format code using hugegraph code style

imbajin reviewed Mar 14, 2022

View reviewed changes

Update CachedGraphTransaction.java

5751a1e

enhance the comment

imbajin changed the title ~~Fix edge cache issue~~ fix: edge cache not clear when update or delete associated vertex Mar 14, 2022

imbajin changed the title ~~fix: edge cache not clear when update or delete associated vertex~~ fix(core): edge cache not clear when update or delete associated vertex Mar 14, 2022

imbajin previously approved these changes Mar 14, 2022

View reviewed changes

javeme previously approved these changes Mar 14, 2022

View reviewed changes

Update CachedGraphTransactionTest.java

fdbe63a

remove one blank line

sunlujing dismissed stale reviews from javeme and imbajin via fdbe63a March 14, 2022 07:18

github-actions bot added a commit that referenced this pull request Mar 14, 2022

@sunlujing has signed the CLA from Pull Request #1780

6a72b07

imbajin approved these changes Mar 14, 2022

View reviewed changes

javeme merged commit 58b3ebe into apache:master Mar 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): edge cache not clear when update or delete associated vertex #1780

fix(core): edge cache not clear when update or delete associated vertex #1780

sunlujing commented Mar 12, 2022 •

edited by javeme

Loading

CLAassistant commented Mar 12, 2022 •

edited

Loading

codecov bot commented Mar 14, 2022 •

edited

Loading

imbajin left a comment •

edited

Loading

imbajin Mar 14, 2022

imbajin Mar 14, 2022

imbajin Mar 14, 2022

imbajin Mar 14, 2022

javeme left a comment

javeme Mar 14, 2022

javeme Mar 14, 2022

javeme Mar 14, 2022

sunlujing commented Mar 14, 2022

imbajin Mar 14, 2022 •

edited

Loading

sunlujing Mar 14, 2022

javeme Mar 14, 2022

sunlujing Mar 14, 2022

github-actions bot commented Mar 14, 2022 •

edited

Loading

sunlujing commented Mar 14, 2022

sunlujing commented Mar 14, 2022

		import com.baidu.hugegraph.structure.HugeEdge;
		import com.baidu.hugegraph.structure.HugeVertexProperty;

fix(core): edge cache not clear when update or delete associated vertex #1780

fix(core): edge cache not clear when update or delete associated vertex #1780

Conversation

sunlujing commented Mar 12, 2022 • edited by javeme Loading

CLAassistant commented Mar 12, 2022 • edited Loading

codecov bot commented Mar 14, 2022 • edited Loading

Codecov Report

imbajin left a comment • edited Loading

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

javeme 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

sunlujing commented Mar 14, 2022

imbajin Mar 14, 2022 • edited Loading

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

github-actions bot commented Mar 14, 2022 • edited Loading

sunlujing commented Mar 14, 2022

sunlujing commented Mar 14, 2022

sunlujing commented Mar 12, 2022 •

edited by javeme

Loading

CLAassistant commented Mar 12, 2022 •

edited

Loading

codecov bot commented Mar 14, 2022 •

edited

Loading

imbajin left a comment •

edited

Loading

imbajin Mar 14, 2022 •

edited

Loading

github-actions bot commented Mar 14, 2022 •

edited

Loading