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

fix ScyllaDB: lost page results due to not fetched the entire page #1407

Merged
merged 3 commits into from
Apr 26, 2021

Conversation

javeme
Copy link
Contributor

@javeme javeme commented Mar 30, 2021

fix #1340

Change-Id: Ib89d9790af4236a137d9215e27f0d17f0de10afd

fix #1340

Change-Id: Ib89d9790af4236a137d9215e27f0d17f0de10afd
@javeme
Copy link
Contributor Author

javeme commented Mar 30, 2021

@javeme
Copy link
Contributor Author

javeme commented Mar 31, 2021

ci error of cassandra backend:

[ERROR] Tests run: 684, Failures: 1, Errors: 0, Skipped: 21, Time elapsed: 513.523 s <<< FAILURE! - in com.baidu.hugegraph.core.CoreTestSuite
[ERROR] testAddEdgeWithTtlAndTtlStartTime(com.baidu.hugegraph.core.EdgeCoreTest)  Time elapsed: 1.872 s  <<< FAILURE!
java.lang.AssertionError
	at com.baidu.hugegraph.core.EdgeCoreTest.testAddEdgeWithTtlAndTtlStartTime(EdgeCoreTest.java:754)

It's ok after retry. seems the reason is: current testAddEdgeWithTtlAndTtlStartTime use the timestamp of the previous test case used.

@codecov
Copy link

codecov bot commented Mar 31, 2021

Codecov Report

Merging #1407 (92c91dc) into master (f57ad0d) will increase coverage by 0.33%.
The diff coverage is 65.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1407      +/-   ##
============================================
+ Coverage     61.89%   62.22%   +0.33%     
- Complexity     5811     5857      +46     
============================================
  Files           386      381       -5     
  Lines         32192    32135      -57     
  Branches       4497     4504       +7     
============================================
+ Hits          19924    19995      +71     
+ Misses        10230    10089     -141     
- Partials       2038     2051      +13     
Impacted Files Coverage Δ Complexity Δ
.../hugegraph/backend/store/rocksdb/RocksDBTable.java 71.53% <0.00%> (ø) 37.00 <0.00> (ø)
...egraph/backend/store/cassandra/CassandraTable.java 79.16% <60.00%> (+3.28%) 87.00 <8.00> (+7.00)
...ackend/store/cassandra/CassandraEntryIterator.java 72.46% <63.15%> (-14.21%) 21.00 <3.00> (+1.00) ⬇️
...egraph/backend/serializer/BinaryEntryIterator.java 85.18% <71.42%> (-0.53%) 21.00 <4.00> (-1.00)
.../hugegraph/backend/store/BackendEntryIterator.java 64.78% <72.22%> (+0.85%) 20.00 <4.00> (+2.00)
...va/com/baidu/hugegraph/backend/page/PageState.java 97.22% <100.00%> (+0.07%) 14.00 <1.00> (ø)
...ava/com/baidu/hugegraph/api/profile/GraphsAPI.java 20.75% <0.00%> (-3.69%) 0.00% <0.00%> (ø%)
...ph/backend/store/AbstractBackendStoreProvider.java 87.91% <0.00%> (-3.66%) 25.00% <0.00%> (+1.00%) ⬇️
...gegraph/backend/store/rocksdb/RocksDBFeatures.java 78.26% <0.00%> (-3.56%) 18.00% <0.00%> (ø%)
...in/java/com/baidu/hugegraph/StandardHugeGraph.java 68.83% <0.00%> (-1.56%) 119.00% <0.00%> (ø%)
... and 53 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 f57ad0d...92c91dc. Read the comment docs.

@javeme
Copy link
Contributor Author

javeme commented Apr 7, 2021

TODO: ensure return the full page to users which scylla fetched to local.

zhoney
zhoney previously approved these changes Apr 14, 2021
Change-Id: Ife31d04509c1815793b07badc101bde27ba1299c
@@ -72,6 +72,7 @@ public String toString() {
}

public static PageState fromString(String page) {
E.checkNotNull("page", "page");
Copy link
Contributor

Choose a reason for hiding this comment

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

E.checkNotNull(page, "page");

Change-Id: I8abe05cbdc95c28bc384e73d554a30300042f3bb
@Linary Linary merged commit 706ffe5 into master Apr 26, 2021
@Linary Linary deleted the fix-scylladb-missing-page-results branch April 26, 2021 06:26
zhoney pushed a commit that referenced this pull request May 27, 2021
…1407)

fix #1340

* return page offset if not return local fetched data to users

Change-Id: I8abe05cbdc95c28bc384e73d554a30300042f3bb
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.

ScyllaDB作为后端存储,分页查询有截断,触发Unexpected fetched page size错误
3 participants