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 bug: count sql statement leak #1640

Merged
merged 4 commits into from
Nov 10, 2021
Merged

Conversation

z7658329
Copy link
Member

Fix bug: mysql count sql statement leak

fix #1626

Fix bug: mysql count sql statement leak

fix apache#1626
@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #1640 (851a8b1) into master (cc509f9) will increase coverage by 0.41%.
The diff coverage is 40.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1640      +/-   ##
============================================
+ Coverage     62.83%   63.24%   +0.41%     
- Complexity     6737     6759      +22     
============================================
  Files           421      421              
  Lines         34645    34646       +1     
  Branches       4798     4798              
============================================
+ Hits          21770    21913     +143     
+ Misses        10606    10460     -146     
- Partials       2269     2273       +4     
Impacted Files Coverage Δ
...om/baidu/hugegraph/backend/store/BackendEntry.java 72.22% <ø> (+2.77%) ⬆️
...aidu/hugegraph/backend/store/mysql/MysqlTable.java 0.00% <0.00%> (ø)
.../hugegraph/backend/store/rocksdb/RocksDBTable.java 70.99% <50.00%> (ø)
.../baidu/hugegraph/backend/query/ConditionQuery.java 86.11% <0.00%> (+0.27%) ⬆️
...c/main/java/com/baidu/hugegraph/task/HugeTask.java 72.30% <0.00%> (+0.30%) ⬆️
...a/com/baidu/hugegraph/backend/query/Condition.java 80.14% <0.00%> (+0.72%) ⬆️
.../backend/store/cassandra/CassandraSessionPool.java 57.14% <0.00%> (+1.02%) ⬆️
...egraph/backend/store/cassandra/CassandraShard.java 52.77% <0.00%> (+1.85%) ⬆️
.../backend/store/scylladb/ScyllaDBStoreProvider.java 94.59% <0.00%> (+94.59%) ⬆️
... and 2 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 cc509f9...851a8b1. Read the comment docs.

@z7658329
Copy link
Member Author

image

与预期符合每次创建一个新连接,都会close该资源。

监控monitor代码:

image

image

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

LGTM!
we can specify the title like fix resultset leak for count query

@@ -340,6 +340,8 @@ public Number queryNumber(Session session, Query query) {
return IteratorUtils.of(rs.resultSet().getLong(1));
} catch (SQLException e) {
throw new BackendException(e);
} finally {
rs.close();
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, can you also fix rocksdb:

      try (BackendColumnIterator results = this.queryBy(session, query)) {
          if (results instanceof Countable) {
              return ((Countable) results).count();
          }
          return IteratorUtils.count(results);
      }

@z7658329 z7658329 changed the title Update MysqlTable.java Fix bug: mysql count sql statement leak Nov 10, 2021
@z7658329 z7658329 changed the title Fix bug: mysql count sql statement leak Fix bug: count sql statement leak Nov 10, 2021
Fix bug: count sql statement leak
@javeme javeme merged commit 299449b into apache:master Nov 10, 2021
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.

[Bug] mysql 后端运行一段时间可能内存泄漏
3 participants