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

set indexlabel invalid if create or rebuild failed #1226

Merged
merged 6 commits into from
Oct 29, 2020
Merged

Conversation

zhoney
Copy link
Contributor

@zhoney zhoney commented Oct 27, 2020

also check if indexlabel invalid when query by index

Change-Id: If890b3bd8c441b63642a2006479cf4ccca9ddf67

also check if indexlabel invalid when query by index

Change-Id: If890b3bd8c441b63642a2006479cf4ccca9ddf67
@codecov
Copy link

codecov bot commented Oct 27, 2020

Codecov Report

Merging #1226 into master will decrease coverage by 0.78%.
The diff coverage is 60.31%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1226      +/-   ##
============================================
- Coverage     65.76%   64.97%   -0.79%     
- Complexity     5752     5762      +10     
============================================
  Files           360      367       +7     
  Lines         29593    30062     +469     
  Branches       4180     4242      +62     
============================================
+ Hits          19462    19533      +71     
- Misses         8152     8548     +396     
- Partials       1979     1981       +2     
Impacted Files Coverage Δ Complexity Δ
...aidu/hugegraph/schema/builder/AbstractBuilder.java 89.65% <0.00%> (-6.65%) 14.00 <0.00> (ø)
...idu/hugegraph/job/schema/RebuildIndexCallable.java 76.82% <43.75%> (-8.11%) 21.00 <0.00> (ø)
...du/hugegraph/schema/builder/IndexLabelBuilder.java 87.34% <50.00%> (-0.80%) 115.00 <0.00> (ø)
.../com/baidu/hugegraph/type/define/SchemaStatus.java 80.00% <60.00%> (-7.50%) 8.00 <3.00> (+3.00) ⬇️
...hugegraph/job/schema/IndexLabelRemoveCallable.java 77.27% <62.50%> (-11.62%) 4.00 <0.00> (ø)
.../hugegraph/job/schema/EdgeLabelRemoveCallable.java 80.00% <66.66%> (-10.48%) 5.00 <0.00> (ø)
...ugegraph/job/schema/VertexLabelRemoveCallable.java 83.87% <66.66%> (-8.73%) 7.00 <0.00> (ø)
...du/hugegraph/backend/tx/GraphIndexTransaction.java 81.32% <100.00%> (+0.15%) 203.00 <1.00> (+2.00)
...m/baidu/hugegraph/backend/tx/GraphTransaction.java 80.19% <100.00%> (ø) 307.00 <0.00> (ø)
...ava/com/baidu/hugegraph/serializer/Serializer.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
... and 20 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 0e4eaa0...d20e577. Read the comment docs.

@@ -1349,6 +1354,13 @@ private static NoIndexException noIndexException(HugeGraph graph,
name, String.join("/", mismatched));
}

private static void validIndexLabel(IndexLabel indexLabel) {
Copy link
Contributor

Choose a reason for hiding this comment

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

validate

@@ -1349,6 +1354,13 @@ private static NoIndexException noIndexException(HugeGraph graph,
name, String.join("/", mismatched));
}

private static void validIndexLabel(IndexLabel indexLabel) {
if (indexLabel.status() == SchemaStatus.INVALID) {
throw new HugeException("The label index %s is invalid",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't query by label index '%s' due to it is in invalid status

@@ -158,6 +166,9 @@ private void removeIndex(Collection<Id> indexLabelIds) {
try {
locks.lockWrites(LockUtil.INDEX_LABEL_DELETE, indexLabelIds);
graphTx.removeIndex(il);
} catch (Throwable t) {
il.status(SchemaStatus.INVALID);
Copy link
Contributor

Choose a reason for hiding this comment

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

does save to backend store? seems need to call schemaTx.updateSchemaStatus()

return new IndexLabel.CreatedIndexLabel(indexLabel,
rebuildTask);
} catch (Throwable t) {
indexLabel.status(SchemaStatus.INVALID);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

rebuildTask);
} catch (Throwable t) {
indexLabel.status(SchemaStatus.INVALID);
throw t;
Copy link
Contributor

Choose a reason for hiding this comment

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

rename t to e

Change-Id: Ib8ee461872a463480fdc03faf3240a4254cc66f9
Linary
Linary previously approved these changes Oct 27, 2020
indexLabel.status(SchemaStatus.INVALID);
throw t;
this.graph().addIndexLabel(schemaLabel, indexLabel);
Copy link
Contributor

Choose a reason for hiding this comment

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

schemaTx.updateSchemaStatus(indexLabel, SchemaStatus.INVALID);

Change-Id: I57331425089e18582f5938536c5026e703ce34c6
Linary
Linary previously approved these changes Oct 28, 2020
@@ -231,8 +231,7 @@ private boolean hasSameProperties(IndexLabel existedIndexLabel) {
return new IndexLabel.CreatedIndexLabel(indexLabel,
rebuildTask);
} catch (Throwable e) {
indexLabel.status(SchemaStatus.INVALID);
this.graph().addIndexLabel(schemaLabel, indexLabel);
this.updateSchemaStatus(indexLabel, SchemaStatus.INVALID);
Copy link
Contributor

Choose a reason for hiding this comment

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

also consider removeSubIndex() at line 207

Change-Id: I6aeaabe6325c336e947fa0d09a824a4138473fa4
} catch (Throwable e) {
schemaTx.updateSchemaStatus(indexLabel, SchemaStatus.INVALID);
throw e;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

should update status to INVALID when failure to remove vertex/edge label?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no need. Fail to remove vertex/edge label will lead to partial delete of vertices/edges. vertex/edge label need to be still valid for remaining vertices/edges

removeIndexLabelFromBaseLabel(schemaTx, indexLabel);
removeSchema(schemaTx, indexLabel);
// Should commit changes to backend store
// before release delete lock
Copy link
Contributor

Choose a reason for hiding this comment

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

use "/*"

Change-Id: I855e51feda284ba74522cd98f11b11975dcb80d9
Change-Id: I7537bd5aff2b43e8a3ba26c88f83761889386ff3
@javeme javeme merged commit 406cfe0 into master Oct 29, 2020
@javeme javeme deleted the il-create-failed branch October 29, 2020 03:13
tmljob pushed a commit to tmljob/hugegraph that referenced this pull request Dec 10, 2020
* set indexlabel invalid if create or rebuild failed
* also check if indexlabel invalid when query by index
* set vl/el to UNDELETED if fail to delete

Change-Id: If890b3bd8c441b63642a2006479cf4ccca9ddf67
tmljob pushed a commit to tmljob/hugegraph that referenced this pull request Dec 10, 2020
* set indexlabel invalid if create or rebuild failed
* also check if indexlabel invalid when query by index
* set vl/el to UNDELETED if fail to delete

Change-Id: If890b3bd8c441b63642a2006479cf4ccca9ddf67
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants