kbendick commented on a change in pull request #2649:
URL: https://github.com/apache/iceberg/pull/2649#discussion_r641731970
##########
File path: core/src/main/java/org/apache/iceberg/ClientPoolImpl.java
##########
@@ -140,4 +148,12 @@ private void release(C client) {
public int poolSize() {
return poolSize;
}
+
+ public static AtomicInteger openCount() {
+ return openCount;
+ }
+
+ public static AtomicInteger closeCount() {
+ return closeCount;
Review comment:
Would it make more sense to call `.get()` in the return statement and
just return an `Integer`?
It seems the only places in the code that these functions are called,
`.get()` is called immediately. It seems to me that calling `.get()` would
provide the benefit of a defensive copy to truly leave these private.
##########
File path: core/src/main/java/org/apache/iceberg/ClientPoolImpl.java
##########
@@ -36,6 +38,10 @@
private volatile int currentSize;
private boolean closed;
+ private static volatile AtomicInteger openCount = new AtomicInteger(0);
Review comment:
I'm a little confused on the variables as well.
If I'm not mistaken, it's the total number of connections ever opened and
the total number of connections closed? Looking at the declaration, I would
have thought that it was the current number of open connections, not the number
of connections ever opened.
My only indication that it might be otherwise came from the unit test -
https://github.com/apache/iceberg/pull/2649/files#diff-8b2035adec2ca6d247f624aeb6dbb2cc3789bd188541c500de4319b969813758R78-R80
```java
private void waitingForCleanup() {
long startTime = System.currentTimeMillis();
while ((ClientPoolImpl.openCount().get() !=
ClientPoolImpl.closeCount().get())
```
Maybe you could add a small comment?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
