You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airavata.apache.org by ma...@apache.org on 2020/01/10 17:54:11 UTC

[airavata] branch develop updated: AIRAVATA-3186 Separate property for logging abandoned pooled clients

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

machristie pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/airavata.git


The following commit(s) were added to refs/heads/develop by this push:
     new c51a5b5  AIRAVATA-3186 Separate property for logging abandoned pooled clients
     new cbcbdee  Merge branch 'AIRAVATA-3186-slf4j-log-abandoned' into develop
c51a5b5 is described below

commit c51a5b5b3d91404747595a5e333190622f830232
Author: Marcus Christie <ma...@apache.org>
AuthorDate: Fri Jan 10 12:52:03 2020 -0500

    AIRAVATA-3186 Separate property for logging abandoned pooled clients
    
    This allows use to keep enabled the automatic removal of abandoned pooled thrift
    clients in production while turning off logging a stacktrace, which has
    a performance overhead.
---
 .../scigap/develop/group_vars/all/vars.yml         |  1 +
 .../scigap/production/group_vars/all/vars.yml      |  1 +
 .../scigap/staging/group_vars/all/vars.yml         |  1 +
 dev-tools/ansible/roles/api-orch/defaults/main.yml |  1 +
 .../templates/airavata-server.properties.j2        |  1 +
 .../ansible/roles/helix_setup/defaults/main.yml    |  1 +
 .../parser-wm/airavata-server.properties.j2        |  1 +
 .../participant/airavata-server.properties.j2      |  1 +
 .../post-wm/airavata-server.properties.j2          |  1 +
 .../templates/pre-wm/airavata-server.properties.j2 |  1 +
 .../ansible/roles/job_monitor/defaults/main.yml    |  1 +
 .../email-monitor/airavata-server.properties.j2    |  1 +
 .../airavata/common/utils/ApplicationSettings.java |  5 +++
 .../airavata/common/utils/ThriftClientPool.java    |  8 +++-
 .../common/utils/ThriftClientPoolTest.java         | 52 ++++++++++++++++++++++
 .../src/main/resources/airavata-server.properties  |  1 +
 16 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/dev-tools/ansible/inventories/scigap/develop/group_vars/all/vars.yml b/dev-tools/ansible/inventories/scigap/develop/group_vars/all/vars.yml
index fd07982..6cde1aa 100644
--- a/dev-tools/ansible/inventories/scigap/develop/group_vars/all/vars.yml
+++ b/dev-tools/ansible/inventories/scigap/develop/group_vars/all/vars.yml
@@ -175,3 +175,4 @@ parser_storage_resource_id: "pgadev.scigap.org_7ddf28fd-d503-4ff8-bbc5-3279a7c3b
 parser_broker_publisher_id: "ParserProducer"
 
 thrift_client_pool_abandoned_removal_enabled: true
+thrift_client_pool_abandoned_removal_logged: true
diff --git a/dev-tools/ansible/inventories/scigap/production/group_vars/all/vars.yml b/dev-tools/ansible/inventories/scigap/production/group_vars/all/vars.yml
index 924fb36..7cf98b2 100644
--- a/dev-tools/ansible/inventories/scigap/production/group_vars/all/vars.yml
+++ b/dev-tools/ansible/inventories/scigap/production/group_vars/all/vars.yml
@@ -170,3 +170,4 @@ platform_monitor_email_password: "{{ vault_platform_monitor_email_password }}"
 platform_monitor_target_email_addresses: "dimuthu.upeksha2@gmail.com,dwannipu@iu.edu"
 
 thrift_client_pool_abandoned_removal_enabled: true
+thrift_client_pool_abandoned_removal_logged: false
diff --git a/dev-tools/ansible/inventories/scigap/staging/group_vars/all/vars.yml b/dev-tools/ansible/inventories/scigap/staging/group_vars/all/vars.yml
index 2105989..00f3396 100644
--- a/dev-tools/ansible/inventories/scigap/staging/group_vars/all/vars.yml
+++ b/dev-tools/ansible/inventories/scigap/staging/group_vars/all/vars.yml
@@ -185,3 +185,4 @@ parser_storage_resource_id: "pga.staging.scigap.org_aa63ffa0-a99f-4885-8f4b-81e3
 parser_broker_publisher_id: "ParserProducer"
 
 thrift_client_pool_abandoned_removal_enabled: true
+thrift_client_pool_abandoned_removal_logged: true
diff --git a/dev-tools/ansible/roles/api-orch/defaults/main.yml b/dev-tools/ansible/roles/api-orch/defaults/main.yml
index 582a43b..638eb95 100644
--- a/dev-tools/ansible/roles/api-orch/defaults/main.yml
+++ b/dev-tools/ansible/roles/api-orch/defaults/main.yml
@@ -46,3 +46,4 @@ default_registry_password : "admin"
 api_orch_systemd_unit_file: "/etc/systemd/system/apiorch.service"
 
 thrift_client_pool_abandoned_removal_enabled: false
+thrift_client_pool_abandoned_removal_logged: false
diff --git a/dev-tools/ansible/roles/api-orch/templates/airavata-server.properties.j2 b/dev-tools/ansible/roles/api-orch/templates/airavata-server.properties.j2
index 44e717e..21cb2ff 100644
--- a/dev-tools/ansible/roles/api-orch/templates/airavata-server.properties.j2
+++ b/dev-tools/ansible/roles/api-orch/templates/airavata-server.properties.j2
@@ -328,3 +328,4 @@ db_event_manager=org.apache.airavata.db.event.manager.DBEventManagerRunner
 # ThriftClientPool Configuration
 ###########################################################################
 thrift.client.pool.abandoned.removal.enabled={{ thrift_client_pool_abandoned_removal_enabled }}
+thrift.client.pool.abandoned.removal.logged={{ thrift_client_pool_abandoned_removal_logged }}
diff --git a/dev-tools/ansible/roles/helix_setup/defaults/main.yml b/dev-tools/ansible/roles/helix_setup/defaults/main.yml
index a6385b9..ee7e409 100644
--- a/dev-tools/ansible/roles/helix_setup/defaults/main.yml
+++ b/dev-tools/ansible/roles/helix_setup/defaults/main.yml
@@ -54,4 +54,5 @@ kafka_rest_proxy_listener_port: 8082
 local_data_location: "/tmp"
 
 thrift_client_pool_abandoned_removal_enabled: false
+thrift_client_pool_abandoned_removal_logged: false
 ...
diff --git a/dev-tools/ansible/roles/helix_setup/templates/parser-wm/airavata-server.properties.j2 b/dev-tools/ansible/roles/helix_setup/templates/parser-wm/airavata-server.properties.j2
index adb8a28..afb4a5d 100644
--- a/dev-tools/ansible/roles/helix_setup/templates/parser-wm/airavata-server.properties.j2
+++ b/dev-tools/ansible/roles/helix_setup/templates/parser-wm/airavata-server.properties.j2
@@ -64,3 +64,4 @@ post.workflow.manager.loadbalance.clusters=False
 # ThriftClientPool Configuration
 ###########################################################################
 thrift.client.pool.abandoned.removal.enabled={{ thrift_client_pool_abandoned_removal_enabled }}
+thrift.client.pool.abandoned.removal.logged={{ thrift_client_pool_abandoned_removal_logged }}
diff --git a/dev-tools/ansible/roles/helix_setup/templates/participant/airavata-server.properties.j2 b/dev-tools/ansible/roles/helix_setup/templates/participant/airavata-server.properties.j2
index f22eb96..b23da56 100644
--- a/dev-tools/ansible/roles/helix_setup/templates/participant/airavata-server.properties.j2
+++ b/dev-tools/ansible/roles/helix_setup/templates/participant/airavata-server.properties.j2
@@ -99,3 +99,4 @@ enable.streaming.transfer=False
 # ThriftClientPool Configuration
 ###########################################################################
 thrift.client.pool.abandoned.removal.enabled={{ thrift_client_pool_abandoned_removal_enabled }}
+thrift.client.pool.abandoned.removal.logged={{ thrift_client_pool_abandoned_removal_logged }}
diff --git a/dev-tools/ansible/roles/helix_setup/templates/post-wm/airavata-server.properties.j2 b/dev-tools/ansible/roles/helix_setup/templates/post-wm/airavata-server.properties.j2
index 7616f6b..33cb199 100644
--- a/dev-tools/ansible/roles/helix_setup/templates/post-wm/airavata-server.properties.j2
+++ b/dev-tools/ansible/roles/helix_setup/templates/post-wm/airavata-server.properties.j2
@@ -58,3 +58,4 @@ zookeeper.timeout=30000
 # ThriftClientPool Configuration
 ###########################################################################
 thrift.client.pool.abandoned.removal.enabled={{ thrift_client_pool_abandoned_removal_enabled }}
+thrift.client.pool.abandoned.removal.logged={{ thrift_client_pool_abandoned_removal_logged }}
diff --git a/dev-tools/ansible/roles/helix_setup/templates/pre-wm/airavata-server.properties.j2 b/dev-tools/ansible/roles/helix_setup/templates/pre-wm/airavata-server.properties.j2
index 5cdcf7f..e854e8c 100644
--- a/dev-tools/ansible/roles/helix_setup/templates/pre-wm/airavata-server.properties.j2
+++ b/dev-tools/ansible/roles/helix_setup/templates/pre-wm/airavata-server.properties.j2
@@ -55,3 +55,4 @@ zookeeper.timeout=30000
 # ThriftClientPool Configuration
 ###########################################################################
 thrift.client.pool.abandoned.removal.enabled={{ thrift_client_pool_abandoned_removal_enabled }}
+thrift.client.pool.abandoned.removal.logged={{ thrift_client_pool_abandoned_removal_logged }}
diff --git a/dev-tools/ansible/roles/job_monitor/defaults/main.yml b/dev-tools/ansible/roles/job_monitor/defaults/main.yml
index c2c655f..9aa419c 100644
--- a/dev-tools/ansible/roles/job_monitor/defaults/main.yml
+++ b/dev-tools/ansible/roles/job_monitor/defaults/main.yml
@@ -47,4 +47,5 @@ helix_log_max_history: 30
 helix_log_total_size_cap: "1GB"
 
 thrift_client_pool_abandoned_removal_enabled: false
+thrift_client_pool_abandoned_removal_logged: false
 ...
diff --git a/dev-tools/ansible/roles/job_monitor/templates/email-monitor/airavata-server.properties.j2 b/dev-tools/ansible/roles/job_monitor/templates/email-monitor/airavata-server.properties.j2
index 2e50351..99eef24 100644
--- a/dev-tools/ansible/roles/job_monitor/templates/email-monitor/airavata-server.properties.j2
+++ b/dev-tools/ansible/roles/job_monitor/templates/email-monitor/airavata-server.properties.j2
@@ -45,3 +45,4 @@ regserver.server.port={{ registry_port }}
 # ThriftClientPool Configuration
 ###########################################################################
 thrift.client.pool.abandoned.removal.enabled={{ thrift_client_pool_abandoned_removal_enabled }}
+thrift.client.pool.abandoned.removal.logged={{ thrift_client_pool_abandoned_removal_logged }}
diff --git a/modules/commons/src/main/java/org/apache/airavata/common/utils/ApplicationSettings.java b/modules/commons/src/main/java/org/apache/airavata/common/utils/ApplicationSettings.java
index f0a75c3..f78b196 100644
--- a/modules/commons/src/main/java/org/apache/airavata/common/utils/ApplicationSettings.java
+++ b/modules/commons/src/main/java/org/apache/airavata/common/utils/ApplicationSettings.java
@@ -55,6 +55,7 @@ public class ApplicationSettings {
 
     // ThriftClientPool Constants
     private static final String THRIFT_CLIENT_POOL_ABANDONED_REMOVAL_ENABLED = "thrift.client.pool.abandoned.removal.enabled";
+    private static final String THRIFT_CLIENT_POOL_ABANDONED_REMOVAL_LOGGED = "thrift.client.pool.abandoned.removal.logged";
     
     protected static ApplicationSettings INSTANCE;
     public static enum ShutdownStrategy{
@@ -442,6 +443,10 @@ public class ApplicationSettings {
         return Boolean.valueOf(getSetting(THRIFT_CLIENT_POOL_ABANDONED_REMOVAL_ENABLED, "false"));
     }
 
+    public static boolean isThriftClientPoolAbandonedRemovalLogged() {
+        return Boolean.valueOf(getSetting(THRIFT_CLIENT_POOL_ABANDONED_REMOVAL_LOGGED, "false"));
+    }
+
     /**
      * @deprecated use {{@link #getSetting(String)}}
      * @return
diff --git a/modules/commons/src/main/java/org/apache/airavata/common/utils/ThriftClientPool.java b/modules/commons/src/main/java/org/apache/airavata/common/utils/ThriftClientPool.java
index f4980a3..1cbfee6 100644
--- a/modules/commons/src/main/java/org/apache/airavata/common/utils/ThriftClientPool.java
+++ b/modules/commons/src/main/java/org/apache/airavata/common/utils/ThriftClientPool.java
@@ -67,10 +67,14 @@ public class ThriftClientPool<T extends BaseAPI.Client> implements AutoCloseable
         AbandonedConfig abandonedConfig = null;
         if (ApplicationSettings.isThriftClientPoolAbandonedRemovalEnabled()) {
             abandonedConfig = new AbandonedConfig();
-            abandonedConfig.setLogAbandoned(true);
             abandonedConfig.setRemoveAbandonedOnBorrow(true);
             abandonedConfig.setRemoveAbandonedOnMaintenance(true);
-            abandonedConfig.setLogWriter(new PrintWriter(new ErrorLoggingStringWriter()));
+            if (ApplicationSettings.isThriftClientPoolAbandonedRemovalLogged()) {
+                abandonedConfig.setLogAbandoned(true);
+                abandonedConfig.setLogWriter(new PrintWriter(new ErrorLoggingStringWriter()));
+            } else {
+                abandonedConfig.setLogAbandoned(false);
+            }
         }
         this.internalPool = new GenericObjectPool<T>(new ThriftClientFactory(clientFactory, protocolFactory),
                 poolConfig, abandonedConfig);
diff --git a/modules/commons/src/test/java/org/apache/airavata/common/utils/ThriftClientPoolTest.java b/modules/commons/src/test/java/org/apache/airavata/common/utils/ThriftClientPoolTest.java
index 5943fdf..34751cc 100644
--- a/modules/commons/src/test/java/org/apache/airavata/common/utils/ThriftClientPoolTest.java
+++ b/modules/commons/src/test/java/org/apache/airavata/common/utils/ThriftClientPoolTest.java
@@ -104,6 +104,58 @@ public class ThriftClientPoolTest {
         };
     }
 
+    @Test
+    public void testWithAbandonConfigAndAbandonedAndNotLogged() throws TException {
+
+        new Expectations() {
+            {
+                mockClient.getAPIVersion();
+                result = "0.19";
+                mockClient.getInputProtocol().getTransport().isOpen();
+                result = true;
+                mockClient.getOutputProtocol().getTransport().isOpen();
+                result = true;
+            }
+        };
+
+        GenericObjectPoolConfig<BaseAPI.Client> poolConfig = new GenericObjectPoolConfig<>();
+        // timeBetweenEvictionRunsMillis must be positive for abandoned removal on
+        // maintenance to run
+        poolConfig.setTimeBetweenEvictionRunsMillis(1);
+        AbandonedConfig abandonedConfig = new AbandonedConfig();
+        abandonedConfig.setRemoveAbandonedTimeout(1);
+        abandonedConfig.setRemoveAbandonedOnMaintenance(true);
+        abandonedConfig.setLogAbandoned(false);
+        // Setup log writer so we can verify that nothing was logged
+        StringWriter log = new StringWriter();
+        Assert.assertEquals("Initial length of log is 0", 0, log.toString().length());
+        PrintWriter logWriter = new PrintWriter(log);
+        abandonedConfig.setLogWriter(logWriter);
+        ThriftClientPool<BaseAPI.Client> thriftClientPool = new ThriftClientPool<>((protocol) -> mockClient, () -> null,
+                poolConfig, abandonedConfig);
+        thriftClientPool.getResource();
+        try {
+            // Sleep long enough for the client to be considered abandoned
+            Thread.sleep(1001);
+            thriftClientPool.close();
+        } catch (InterruptedException e) {
+            Assert.fail("sleep interrupted");
+        }
+
+        // Verify that nothing was logged
+        Assert.assertEquals(0, log.toString().length());
+
+        new Verifications() {
+            {
+                // Verify client is destroyed when abandoned
+                mockClient.getInputProtocol().getTransport().close();
+                times = 1;
+                mockClient.getOutputProtocol().getTransport().close();
+                times = 1;
+            }
+        };
+    }
+
     /**
      * Just like #{@link #testWithAbandonConfigAndAbandoned()} but using default
      * configuration.
diff --git a/modules/ide-integration/src/main/resources/airavata-server.properties b/modules/ide-integration/src/main/resources/airavata-server.properties
index f3745ec..2ff2e9f 100644
--- a/modules/ide-integration/src/main/resources/airavata-server.properties
+++ b/modules/ide-integration/src/main/resources/airavata-server.properties
@@ -282,3 +282,4 @@ email.based.monitor.password=CHANGEME
 # ThriftClientPool Configuration
 ###########################################################################
 thrift.client.pool.abandoned.removal.enabled=true
+thrift.client.pool.abandoned.removal.logged=true