This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 064a4eb145816fbb91948f3c286aeafc1f7f3933
Author: Alex Heneveld <[email protected]>
AuthorDate: Thu Jun 27 10:51:13 2024 +0100

    improve handling if ssh expiry set to 0/negative/forever
---
 .../brooklyn/location/ssh/SshMachineLocation.java  | 24 ++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git 
a/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java 
b/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
index a1619b7e00..cef74dae72 100644
--- 
a/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
+++ 
b/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
@@ -307,14 +307,26 @@ public class SshMachineLocation extends 
AbstractMachineLocation implements Machi
     }
 
     private LoadingCache<Map<String, ?>, Pool<SshTool>> 
buildSshToolPoolCacheLoader() {
-        // TODO: Appropriate numbers for maximum size and expire after access
-        // At the moment every SshMachineLocation instance creates its own 
pool.
-        // It might make more sense to create one pool and inject it into all 
SshMachineLocations.
+        // Every SshMachineLocation instance creates its own pool.
+        // Expiry is configurable, but max size is not.
+
+        CacheBuilder<Object, Object> db = CacheBuilder.newBuilder();
+
         Duration expiryDuration = getConfig(SSH_CACHE_EXPIRY_DURATION);
-        
-        LoadingCache<Map<String, ?>, Pool<SshTool>> delegate = 
CacheBuilder.newBuilder()
+        if (expiryDuration!=null) {
+            // has a default, so shouldn't normally be null
+            if (!expiryDuration.isPositive()) {
+                // non-positive duration is an error, as it disables cache
+                throw new IllegalArgumentException("Expiry duration must be 
positive if specified. " +
+                        "To trigger an immediate close after use, set " +
+                        
ConfigUtils.prefixedKey(SshTool.BROOKLYN_CONFIG_KEY_PREFIX, 
CLOSE_CONNECTION).getName() + "=true instead.");
+            } else if 
(expiryDuration.isShorterThan(Duration.PRACTICALLY_FOREVER)){
+                db = db.expireAfterAccess(expiryDuration.toNanoseconds(), 
TimeUnit.NANOSECONDS);
+            }
+        }
+
+        LoadingCache<Map<String, ?>, Pool<SshTool>> delegate = db
                 .maximumSize(10)
-                .expireAfterAccess(expiryDuration.toMilliseconds(), 
TimeUnit.MILLISECONDS)
                 .recordStats()
                 .removalListener(new RemovalListener<Map<String, ?>, 
Pool<SshTool>>() {
                     // TODO: Does it matter that this is synchronous? - Can 
closing pools cause long delays?

Reply via email to