You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hugegraph.apache.org by GitBox <gi...@apache.org> on 2022/11/10 02:18:08 UTC
[GitHub] [incubator-hugegraph] vamossagar12 opened a new pull request, #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
vamossagar12 opened a new pull request, #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014
Fix #1983
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] javeme commented on a diff in pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
javeme commented on code in PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#discussion_r1021423594
##########
hugegraph-cassandra/src/main/java/com/baidu/hugegraph/backend/store/cassandra/CassandraTable.java:
##########
@@ -156,6 +156,12 @@ protected <R> Iterator<R> query(Query query,
} catch (DriverException e) {
LOG.debug("Failed to query [{}], detail statement: {}",
query, selects, e);
+ // Closing the iterator
+ try {
+ rs.close();
+ } catch (Exception e2) {
+ LOG.error("Got error {} when closing iterator for query {}", e, query);
Review Comment:
I think we should log e2 here
##########
hugegraph-mysql/src/main/java/com/baidu/hugegraph/backend/store/mysql/MysqlTable.java:
##########
@@ -477,6 +477,12 @@ protected <R> Iterator<R> query(Session session, Query query,
rs.extend(parser.apply(query, results));
}
} catch (SQLException e) {
+ // Closing the iterator
+ try {
+ rs.close();
+ } catch (Exception e2) {
+ LOG.error("Got error {} when closing iterator for query {}", e, query);
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] vamossagar12 commented on a diff in pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on code in PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#discussion_r1021549999
##########
hugegraph-cassandra/src/main/java/com/baidu/hugegraph/backend/store/cassandra/CassandraTable.java:
##########
@@ -156,6 +156,12 @@ protected <R> Iterator<R> query(Query query,
} catch (DriverException e) {
LOG.debug("Failed to query [{}], detail statement: {}",
query, selects, e);
+ // Closing the iterator
+ try {
+ rs.close();
+ } catch (Exception e2) {
+ LOG.error("Got error {} when closing iterator for query {}", e, query);
Review Comment:
sorry about that. Fixed it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] vamossagar12 commented on a diff in pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on code in PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#discussion_r1021007920
##########
hugegraph-cassandra/src/main/java/com/baidu/hugegraph/backend/store/cassandra/CassandraTable.java:
##########
@@ -156,6 +156,12 @@ protected <R> Iterator<R> query(Query query,
} catch (DriverException e) {
LOG.debug("Failed to query [{}], detail statement: {}",
query, selects, e);
+ // Closing the iterator
+ try {
+ rs.close();
+ } catch (Exception closeException) {
+ throw new BackendException("Error closing Iterator during query [%s]", e, query);
Review Comment:
done
##########
hugegraph-mysql/src/main/java/com/baidu/hugegraph/backend/store/mysql/MysqlTable.java:
##########
@@ -477,6 +477,12 @@ protected <R> Iterator<R> query(Session session, Query query,
rs.extend(parser.apply(query, results));
}
} catch (SQLException e) {
+ // closing iterator
+ try {
+ rs.close();
+ } catch (Exception closeException) {
+ throw new BackendException("Error closing Iterator during query [%s]", e, query);
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] vamossagar12 commented on pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#issuecomment-1312925862
> Thanks for your contribution
Thanks @javeme . I made the changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] vamossagar12 commented on pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#issuecomment-1309676659
hi @javeme . Plz review. I noticed a similar issue for MysqlTable so added that as well as part of the PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] codecov[bot] commented on pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#issuecomment-1309709735
# [Codecov](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2014](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5b6ee0b) into [master](https://codecov.io/gh/apache/incubator-hugegraph/commit/b1b12098feb726e46c781a2f171c77558db05fc1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1b1209) will **decrease** coverage by `15.57%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #2014 +/- ##
=============================================
- Coverage 62.41% 46.83% -15.58%
+ Complexity 976 651 -325
=============================================
Files 482 482
Lines 41450 41458 +8
Branches 5896 5896
=============================================
- Hits 25870 19418 -6452
- Misses 13034 19991 +6957
+ Partials 2546 2049 -497
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...egraph/backend/store/cassandra/CassandraTable.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLWNhc3NhbmRyYS9zcmMvbWFpbi9qYXZhL2NvbS9iYWlkdS9odWdlZ3JhcGgvYmFja2VuZC9zdG9yZS9jYXNzYW5kcmEvQ2Fzc2FuZHJhVGFibGUuamF2YQ==) | `0.00% <0.00%> (-80.14%)` | :arrow_down: |
| [...aidu/hugegraph/backend/store/mysql/MysqlTable.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLW15c3FsL3NyYy9tYWluL2phdmEvY29tL2JhaWR1L2h1Z2VncmFwaC9iYWNrZW5kL3N0b3JlL215c3FsL015c3FsVGFibGUuamF2YQ==) | `82.66% <0.00%> (-0.80%)` | :arrow_down: |
| [...du/hugegraph/util/FixedWatchWindowRateLimiter.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLWNvcmUvc3JjL21haW4vamF2YS9jb20vYmFpZHUvaHVnZWdyYXBoL3V0aWwvRml4ZWRXYXRjaFdpbmRvd1JhdGVMaW1pdGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...hugegraph/job/schema/OlapPropertyKeyCreateJob.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLWNvcmUvc3JjL21haW4vamF2YS9jb20vYmFpZHUvaHVnZWdyYXBoL2pvYi9zY2hlbWEvT2xhcFByb3BlcnR5S2V5Q3JlYXRlSm9iLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...hugegraph/job/schema/OlapPropertyKeyRemoveJob.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLWNvcmUvc3JjL21haW4vamF2YS9jb20vYmFpZHUvaHVnZWdyYXBoL2pvYi9zY2hlbWEvT2xhcFByb3BlcnR5S2V5UmVtb3ZlSm9iLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ugegraph/backend/store/memory/InMemoryMetrics.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLWNvcmUvc3JjL21haW4vamF2YS9jb20vYmFpZHUvaHVnZWdyYXBoL2JhY2tlbmQvc3RvcmUvbWVtb3J5L0luTWVtb3J5TWV0cmljcy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...egraph/backend/store/hbase/HbaseStoreProvider.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLWhiYXNlL3NyYy9tYWluL2phdmEvY29tL2JhaWR1L2h1Z2VncmFwaC9iYWNrZW5kL3N0b3JlL2hiYXNlL0hiYXNlU3RvcmVQcm92aWRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...egraph/backend/store/scylladb/ScyllaDBMetrics.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLXNjeWxsYWRiL3NyYy9tYWluL2phdmEvY29tL2JhaWR1L2h1Z2VncmFwaC9iYWNrZW5kL3N0b3JlL3NjeWxsYWRiL1NjeWxsYURCTWV0cmljcy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...graph/backend/store/scylladb/ScyllaDBFeatures.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLXNjeWxsYWRiL3NyYy9tYWluL2phdmEvY29tL2JhaWR1L2h1Z2VncmFwaC9iYWNrZW5kL3N0b3JlL3NjeWxsYWRiL1NjeWxsYURCRmVhdHVyZXMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ackend/store/cassandra/CassandraStoreProvider.java](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVnZWdyYXBoLWNhc3NhbmRyYS9zcmMvbWFpbi9qYXZhL2NvbS9iYWlkdS9odWdlZ3JhcGgvYmFja2VuZC9zdG9yZS9jYXNzYW5kcmEvQ2Fzc2FuZHJhU3RvcmVQcm92aWRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [147 more](https://codecov.io/gh/apache/incubator-hugegraph/pull/2014/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] vamossagar12 commented on a diff in pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on code in PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#discussion_r1021550339
##########
hugegraph-mysql/src/main/java/com/baidu/hugegraph/backend/store/mysql/MysqlTable.java:
##########
@@ -477,6 +477,12 @@ protected <R> Iterator<R> query(Session session, Query query,
rs.extend(parser.apply(query, results));
}
} catch (SQLException e) {
+ // Closing the iterator
+ try {
+ rs.close();
+ } catch (Exception e2) {
+ LOG.error("Got error {} when closing iterator for query {}", e, query);
Review Comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] javeme merged pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
javeme merged PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] vamossagar12 commented on a diff in pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on code in PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#discussion_r1021007989
##########
hugegraph-cassandra/src/main/java/com/baidu/hugegraph/backend/store/cassandra/CassandraTable.java:
##########
@@ -156,6 +156,12 @@ protected <R> Iterator<R> query(Query query,
} catch (DriverException e) {
LOG.debug("Failed to query [{}], detail statement: {}",
query, selects, e);
+ // Closing the iterator
+ try {
+ rs.close();
+ } catch (Exception closeException) {
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hugegraph] javeme commented on a diff in pull request #2014: Fix bug: Closing iterators on errors for CassandraTable and MysqlTable
Posted by GitBox <gi...@apache.org>.
javeme commented on code in PR #2014:
URL: https://github.com/apache/incubator-hugegraph/pull/2014#discussion_r1019019884
##########
hugegraph-cassandra/src/main/java/com/baidu/hugegraph/backend/store/cassandra/CassandraTable.java:
##########
@@ -156,6 +156,12 @@ protected <R> Iterator<R> query(Query query,
} catch (DriverException e) {
LOG.debug("Failed to query [{}], detail statement: {}",
query, selects, e);
+ // Closing the iterator
+ try {
+ rs.close();
+ } catch (Exception closeException) {
+ throw new BackendException("Error closing Iterator during query [%s]", e, query);
Review Comment:
I think it's better to just log.error closeException here and let throw the DriverException at line 165, because closeException is not the main path.
##########
hugegraph-cassandra/src/main/java/com/baidu/hugegraph/backend/store/cassandra/CassandraTable.java:
##########
@@ -156,6 +156,12 @@ protected <R> Iterator<R> query(Query query,
} catch (DriverException e) {
LOG.debug("Failed to query [{}], detail statement: {}",
query, selects, e);
+ // Closing the iterator
+ try {
+ rs.close();
+ } catch (Exception closeException) {
Review Comment:
'e2' name is ok
##########
hugegraph-mysql/src/main/java/com/baidu/hugegraph/backend/store/mysql/MysqlTable.java:
##########
@@ -477,6 +477,12 @@ protected <R> Iterator<R> query(Session session, Query query,
rs.extend(parser.apply(query, results));
}
} catch (SQLException e) {
+ // closing iterator
+ try {
+ rs.close();
+ } catch (Exception closeException) {
+ throw new BackendException("Error closing Iterator during query [%s]", e, query);
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org