Skip to content

Commit c678dad

Browse files
authored
Cleaned up "inactive channel" auth logic
* Cleaned up "inactive channel" auth logic. - close and remove channel from queue on "inactive" notification - set promise to failed if channel is inactive - added cause to SecurityInfoNotReadyException - add host, port, uri to timeout exception message
1 parent 5f6500b commit c678dad

File tree

8 files changed

+50
-9
lines changed

8 files changed

+50
-9
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
77
### Changed
88
- Moved a couple internal log messages to FINE instead of INFO
99
- Cleaned up messaging when can't connect to server
10+
- Modified internal auth logic to avoid "inactive channel: retrying" messages
1011

1112
## [5.4.11] 2023-06-06
1213

driver/src/main/java/oracle/nosql/driver/RetryableException.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ protected RetryableException(String msg) {
2222
super(msg);
2323
}
2424

25+
/**
26+
* @hidden
27+
*/
28+
protected RetryableException(String msg, Throwable cause) {
29+
super(msg, cause);
30+
}
31+
2532
@Override
2633
public boolean okToRetry() {
2734
return true;

driver/src/main/java/oracle/nosql/driver/SecurityInfoNotReadyException.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,8 @@ public class SecurityInfoNotReadyException extends RetryableException {
2121
public SecurityInfoNotReadyException(String msg) {
2222
super(msg);
2323
}
24+
25+
public SecurityInfoNotReadyException(String msg, Throwable cause) {
26+
super(msg, cause);
27+
}
2428
}

driver/src/main/java/oracle/nosql/driver/httpclient/ConnectionPool.java

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import static oracle.nosql.driver.util.LogUtil.logFine;
1111
import static oracle.nosql.driver.util.LogUtil.logInfo;
1212

13+
import java.io.IOException;
1314
import java.util.Map;
1415
import java.util.concurrent.ConcurrentHashMap;
1516
import java.util.concurrent.ConcurrentLinkedDeque;
@@ -237,6 +238,12 @@ public void operationComplete(
237238
continue;
238239
}
239240
} else {
241+
/*
242+
* Note: run() may be executed some time after this method
243+
* returns a promise. So the caller may have to wait a
244+
* few milliseconds for the promise to be completed
245+
* (successfully or not).
246+
*/
240247
loop.execute(new Runnable() {
241248
@Override
242249
public void run() {
@@ -257,23 +264,32 @@ public void run() {
257264
* front of the queue. This class implements a LIFO algorithm to ensure
258265
* that the first, or first few channels on the queue remain active and
259266
* are not subject to inactivity timeouts from the server side.
267+
* Note that inactive released channels will be closed and not
268+
* re-added to the queue.
260269
*/
261270
void release(Channel channel) {
262271
if (!channel.isActive()) {
263272
logFine(logger,
264273
"Inactive channel on release, closing: " + channel);
265274
removeChannel(channel);
275+
} else {
276+
queue.addFirst(channel);
266277
}
267278
updateStats(channel, false);
268-
queue.addFirst(channel);
269279
try { handler.channelReleased(channel); } catch (Exception e) {}
270280
}
271281

272282
/**
273-
* Closes and removes a channel from the pool entirely. This channel
274-
* must not exist in the queue at this time
283+
* Close and remove channel from pool.
284+
* The channel may or may not currently be in the queue.
285+
* This will normally only be called on channels that were acquired and
286+
* found to be inactive or otherwise invalid, but it may also occasionally
287+
* be called by an async netty callback when netty sees that a channel
288+
* has been disconnected or become otherwise inactive. In the latter case,
289+
* the channel is likely still in the queue and will be removed.
275290
*/
276-
private void removeChannel(Channel channel) {
291+
public void removeChannel(Channel channel) {
292+
queue.remove(channel);
277293
stats.remove(channel);
278294
channel.close();
279295
}
@@ -329,6 +345,7 @@ private boolean checkChannel(final Channel channel,
329345
logFine(logger,
330346
"Inactive channel found, closing: " + channel);
331347
removeChannel(channel);
348+
promise.tryFailure(new IOException("inactive channel"));
332349
return true;
333350
}
334351
try {
@@ -371,7 +388,6 @@ int pruneChannels() {
371388
if (!ch.isActive()) {
372389
logFine(logger,
373390
"Channel being pruned due to server close: " + ch);
374-
queue.remove(ch);
375391
removeChannel(ch);
376392
pruned++;
377393
}

driver/src/main/java/oracle/nosql/driver/httpclient/HttpClient.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,11 +282,11 @@ SslContext getSslContext() {
282282
return sslCtx;
283283
}
284284

285-
int getPort() {
285+
public int getPort() {
286286
return port;
287287
}
288288

289-
String getHost() {
289+
public String getHost() {
290290
return host;
291291
}
292292

@@ -446,6 +446,15 @@ public void releaseChannel(Channel channel) {
446446
pool.release(channel);
447447
}
448448

449+
/**
450+
* Close and remove channel from client pool.
451+
*/
452+
public void removeChannel(Channel channel) {
453+
logFine(logger, "closing and removing channel " + channel);
454+
pool.removeChannel(channel);
455+
}
456+
457+
449458
/**
450459
* Sends an HttpRequest, setting up the ResponseHandler as the handler to
451460
* use for the (asynchronous) response.

driver/src/main/java/oracle/nosql/driver/httpclient/HttpClientChannelPoolHandler.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ public void channelInactive(ChannelHandlerContext ctx) {
153153
logFine(client.getLogger(),
154154
"HttpClient " + client.getName() +
155155
", channel " + ctx.channel() + " inactive");
156+
client.removeChannel(ctx.channel());
156157
ctx.fireChannelInactive();
157158
}
158159

driver/src/main/java/oracle/nosql/driver/iam/SecurityTokenSupplier.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ private SecurityToken getSecurityTokenFromIAM() {
232232

233233
return new SecurityToken(securityToken, keyPairSupplier);
234234
} catch (Exception e) {
235-
throw new SecurityInfoNotReadyException(e.getMessage());
235+
throw new SecurityInfoNotReadyException(e.getMessage(), e);
236236
}
237237
}
238238

driver/src/main/java/oracle/nosql/driver/util/HttpRequestUtil.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,10 @@ private static HttpResponse doRequest(HttpClient httpClient,
288288
throw new RuntimeException(
289289
"Unable to execute request: ", ee);
290290
} catch (TimeoutException te) {
291-
throw new RuntimeException("Timeout exception: ", te);
291+
throw new RuntimeException("Timeout exception: host=" +
292+
httpClient.getHost() + " port=" +
293+
httpClient.getPort() + " uri=" +
294+
uri, te);
292295
} catch (Throwable t) {
293296
/*
294297
* this is likely an exception from Netty, perhaps a bad

0 commit comments

Comments
 (0)