You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "lancelly (via GitHub)" <gi...@apache.org> on 2023/05/30 14:24:52 UTC
[GitHub] [iotdb] lancelly opened a new pull request, #9992: Release resource of FI after all drivers have been closed
lancelly opened a new pull request, #9992:
URL: https://github.com/apache/iotdb/pull/9992
We can release resource of FI until all drivers have been closed, otherwise there could be memory leak/file handle leak.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a diff in pull request #9992: Release resource of FI after all drivers have been closed
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 commented on code in PR #9992:
URL: https://github.com/apache/iotdb/pull/9992#discussion_r1210978450
##########
server/src/main/java/org/apache/iotdb/db/mpp/execution/fragment/FragmentInstanceContext.java:
##########
@@ -352,6 +355,26 @@ private void addFilePathToMap(TsFileResource tsFile, boolean isClosed) {
}
}
+ public void initializeNumOfDrivers(Integer numOfDrivers) {
Review Comment:
```suggestion
public void initializeNumOfDrivers(int numOfDrivers) {
```
##########
server/src/main/java/org/apache/iotdb/db/mpp/execution/fragment/FragmentInstanceContext.java:
##########
@@ -352,6 +355,26 @@ private void addFilePathToMap(TsFileResource tsFile, boolean isClosed) {
}
}
+ public void initializeNumOfDrivers(Integer numOfDrivers) {
+ // initialize with the num of Drivers
+ allDriversClosed = new CountDownLatch(numOfDrivers);
+ }
+
+ public void decrementNumOfUnClosedDriver() {
+ allDriversClosed.countDown();
+ }
+
+ public void releaseResourceWhenAllDriversAreClosed() {
+ try {
+ allDriversClosed.await();
+ } catch (InterruptedException e) {
+ LOGGER.warn(
+ "Interrupted when await on allDriversClosed, FragmentInstance Id is {}", this.getId());
+ throw new RuntimeException(e);
+ }
Review Comment:
```suggestion
while (true) {
try {
allDriversClosed.await();
break;
} catch (InterruptedException e) {
LOGGER.warn(
"Interrupted when await on allDriversClosed, FragmentInstance Id is {}", this.getId());
}
}
```
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] lancelly commented on a diff in pull request #9992: Release resource of FI after all drivers have been closed
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #9992:
URL: https://github.com/apache/iotdb/pull/9992#discussion_r1210984053
##########
server/src/main/java/org/apache/iotdb/db/mpp/execution/fragment/FragmentInstanceContext.java:
##########
@@ -352,6 +355,26 @@ private void addFilePathToMap(TsFileResource tsFile, boolean isClosed) {
}
}
+ public void initializeNumOfDrivers(Integer numOfDrivers) {
+ // initialize with the num of Drivers
+ allDriversClosed = new CountDownLatch(numOfDrivers);
+ }
+
+ public void decrementNumOfUnClosedDriver() {
+ allDriversClosed.countDown();
+ }
+
+ public void releaseResourceWhenAllDriversAreClosed() {
+ try {
+ allDriversClosed.await();
+ } catch (InterruptedException e) {
+ LOGGER.warn(
+ "Interrupted when await on allDriversClosed, FragmentInstance Id is {}", this.getId());
+ throw new RuntimeException(e);
+ }
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 merged pull request #9992: Release resource of FI after all drivers have been closed
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 merged PR #9992:
URL: https://github.com/apache/iotdb/pull/9992
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] lancelly commented on a diff in pull request #9992: Release resource of FI after all drivers have been closed
Posted by "lancelly (via GitHub)" <gi...@apache.org>.
lancelly commented on code in PR #9992:
URL: https://github.com/apache/iotdb/pull/9992#discussion_r1210984128
##########
server/src/main/java/org/apache/iotdb/db/mpp/execution/fragment/FragmentInstanceContext.java:
##########
@@ -352,6 +355,26 @@ private void addFilePathToMap(TsFileResource tsFile, boolean isClosed) {
}
}
+ public void initializeNumOfDrivers(Integer numOfDrivers) {
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org