[ 
https://issues.apache.org/jira/browse/GEODE-3570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16337867#comment-16337867
 ] 

ASF GitHub Bot commented on GEODE-3570:
---------------------------------------

pivotal-jbarrett closed pull request #191:  GEODE-3570: Standardizes time 
values.
URL: https://github.com/apache/geode-native/pull/191
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/clicache/src/PoolFactory.hpp b/clicache/src/PoolFactory.hpp
index 25ebafeb..a9c891e6 100644
--- a/clicache/src/PoolFactory.hpp
+++ b/clicache/src/PoolFactory.hpp
@@ -75,11 +75,11 @@ namespace Apache
         /// </remarks>
         /// <param>
         /// loadConditioningInterval the connection lifetime
-        /// A value of -1 disables load conditioning.
+        /// A value of 0 disables load conditioning.
         /// </param>
         /// <exception>
         /// throws IllegalArgumentException if connectionLifetime
-        /// is less than -1.
+        /// is less than 0.
         /// </exception>
         PoolFactory^ SetLoadConditioningInterval(TimeSpan 
loadConditioningInterval);
 
@@ -156,7 +156,7 @@ namespace Apache
         /// </remarks>
         /// <param>
         /// idleTimeout The amount of time that an idle connection
-        /// should live before expiring. -1 indicates that connections should 
never expire.
+        /// should live before expiring. 0 indicates that connections should 
never expire.
         /// </param>
         /// <exception>
         /// throws IllegalArgumentException if idleTimout is less than 0.
@@ -213,7 +213,7 @@ namespace Apache
         /// </summary>
         /// <remarks>
         /// Doing this allows gfmon to monitor clients.
-        /// A value of -1 disables the sending of client statistics
+        /// A value of 0 disables the sending of client statistics
         /// to the server.
         /// </remarks>
         /// <param>
@@ -222,7 +222,7 @@ namespace Apache
         /// </param>
         /// <exception>
         /// throws IllegalArgumentException if statisticInterval
-        /// is less than -1.
+        /// is less than 0.
         /// </exception>
         PoolFactory^ SetStatisticInterval(TimeSpan statisticInterval);
 
diff --git a/cppcache/include/geode/PoolFactory.hpp 
b/cppcache/include/geode/PoolFactory.hpp
index b19411dd..f77bf598 100644
--- a/cppcache/include/geode/PoolFactory.hpp
+++ b/cppcache/include/geode/PoolFactory.hpp
@@ -134,7 +134,8 @@ class _GEODE_EXPORT PoolFactory {
 
   /**
    * The default frequency that client statistics are sent to the server.
-   * <p>Current value: <code>-1</code> (disabled).
+   * <p>Current value: <code>std::chrono::milliseconds::zero()</code>
+   * (disabled).
    */
   static const std::chrono::milliseconds DEFAULT_STATISTIC_INTERVAL;
 
@@ -204,23 +205,25 @@ class _GEODE_EXPORT PoolFactory {
    * @param connectionTimeout is the connection timeout
    *
    * @return a reference to <code>this</code>
-   * @throws std::invalid_argument if <code>connectionTimeout</code>
-   * is less than or equal to <code>0</code>.
+   * @throws IllegalArgumentException if <code>connectionTimeout</code>
+   * is less than or equal to <code>std::chrono::milliseconds::zero()</code>.
    */
-  PoolFactory& setFreeConnectionTimeout(std::chrono::milliseconds 
connectionTimeout);
+  PoolFactory& setFreeConnectionTimeout(
+      std::chrono::milliseconds connectionTimeout);
 
   /**
    * Sets the load conditioning interval for this pool.
    * This interval controls how frequently the pool will check to see if
    * a connection to a given server should be moved to a different
    * server to improve the load balance.
-   * <p>A value of <code>-1</code> disables load conditioning
+   * <p>A value of <code>std::chrono::milliseconds::zero()</code> disables load
+   * conditioning.
    *
    * @param loadConditioningInterval is the connection lifetime
    *
    * @return a reference to <code>this</code>
-   * @throws std::invalid_argument if <code>connectionLifetime</code>
-   * is less than <code>-1</code>.
+   * @throws IllegalArgumentException if <code>connectionLifetime</code>
+   * is less than <code>std::chrono::milliseconds::zero()</code>.
    */
   PoolFactory& setLoadConditioningInterval(
       std::chrono::milliseconds loadConditioningInterval);
@@ -267,8 +270,8 @@ class _GEODE_EXPORT PoolFactory {
    * @param timeout duration to wait for a response from a
    * server
    * @return a reference to <code>this</code>
-   * @throws std::invalid_argument if <code>timeout</code>
-   * is less than or equal to <code>0</code>.
+   * @throws IllegalArgumentException if <code>timeout</code>
+   * is less than or equal to <code>std::chrono::milliseconds::zero()</code>.
    */
   PoolFactory& setReadTimeout(std::chrono::milliseconds timeout);
 
@@ -313,8 +316,8 @@ class _GEODE_EXPORT PoolFactory {
    *
    * @param idleTimeout is the duration that an idle connection
    * should live no less than before expiring, actual time may be longer
-   * depending on clock resolution. A duration less than 0 indicates
-   * that connections should never expire.
+   * depending on clock resolution. A duration 
std::chrono::milliseconds::zero()
+   * indicates that connections should never expire.
    * @return a reference to <code>this</code>
    */
   PoolFactory& setIdleTimeout(std::chrono::milliseconds);
@@ -343,7 +346,7 @@ class _GEODE_EXPORT PoolFactory {
    *
    * @param pingInterval is the amount of time  between pings.
    * @return a reference to <code>this</code>
-   * @throws std::invalid_argument if <code>pingInterval</code>
+   * @throws IllegalArgumentException if <code>pingInterval</code>
    * is less than <code>0</code>.
    *
    * @see CacheServer#setMaximumTimeBetweenPings(int)
@@ -352,7 +355,7 @@ class _GEODE_EXPORT PoolFactory {
 
   /**
    * The frequency with which client updates the locator list. To disable this
-   * set its value to 0.
+   * set its value to std::chrono::milliseconds::zero().
    *
    * @param updateLocatorListInterval is the amount of time
    * between checking locator list at locator.
@@ -364,17 +367,18 @@ class _GEODE_EXPORT PoolFactory {
   /**
    * The frequency with which the client statistics must be sent to the server.
    * Doing this allows <code>GFMon</code> to monitor clients.
-   * <p>A value of <code>-1</code> disables the sending of client statistics
-   * to the server.
+   * <p>A value of <code>std::chrono::milliseconds::zero()</code> disables the
+   * sending of client statistics to the server.
    *
    * @param statisticInterval is the amount of time between
    * sends of client statistics to the server.
    *
    * @return a reference to <code>this</code>
-   * @throws std::invalid_argument if <code>statisticInterval</code>
-   * is less than <code>-1</code>.
+   * @throws IllegalArgumentException if <code>statisticInterval</code>
+   * is less than <code>std::chrono::milliseconds::zero()</code>.
    */
-  PoolFactory& setStatisticInterval(std::chrono::milliseconds 
statisticInterval);
+  PoolFactory& setStatisticInterval(
+      std::chrono::milliseconds statisticInterval);
 
   /**
    * Configures the group which contains all the servers that this pool 
connects
@@ -456,7 +460,7 @@ class _GEODE_EXPORT PoolFactory {
    * @param messageTrackingTimeout is the duration to set the timeout to.
    * @return a reference to <code>this</code>
    *
-   * @throws std::invalid_argument if <code>messageTrackingTimeout</code>
+   * @throws IllegalArgumentException if <code>messageTrackingTimeout</code>
    * is less than or equal to <code>0</code>.
 
    */
@@ -470,11 +474,12 @@ class _GEODE_EXPORT PoolFactory {
    * @param ackInterval is the duration to wait before sending  event
    * acknowledgements.
    *
-   * @throws std::invalid_argument if <code>ackInterval</code>
+   * @throws IllegalArgumentException if <code>ackInterval</code>
    * is less than or equal to <code>0</code>.
    * @return a reference to <code>this</code>
    */
-  PoolFactory& setSubscriptionAckInterval(std::chrono::milliseconds 
ackInterval);
+  PoolFactory& setSubscriptionAckInterval(
+      std::chrono::milliseconds ackInterval);
 
   /**
    * Sets whether Pool is in multi user secure mode.
diff --git a/cppcache/include/geode/RegionAttributes.hpp 
b/cppcache/include/geode/RegionAttributes.hpp
index db9493e8..72af4b1e 100644
--- a/cppcache/include/geode/RegionAttributes.hpp
+++ b/cppcache/include/geode/RegionAttributes.hpp
@@ -342,12 +342,13 @@ class _GEODE_EXPORT RegionAttributes : public 
Serializable {
   void setConcurrencyChecksEnabled(bool enable);
 
   inline bool getEntryExpiryEnabled() const {
-    return (m_entryTimeToLive.count() != 0 || m_entryIdleTimeout.count() != 0);
+    return (m_entryTimeToLive > std::chrono::seconds::zero() ||
+            m_entryIdleTimeout > std::chrono::seconds::zero());
   }
 
   inline bool getRegionExpiryEnabled() const {
-    return (m_regionTimeToLive.count() != 0 ||
-            m_regionIdleTimeout.count() != 0);
+    return (m_regionTimeToLive > std::chrono::seconds::zero() ||
+            m_regionIdleTimeout > std::chrono::seconds::zero());
   }
 
   // will be created by the factory
diff --git a/cppcache/src/CacheTransactionManagerImpl.cpp 
b/cppcache/src/CacheTransactionManagerImpl.cpp
index f6925fe7..537d9bdb 100644
--- a/cppcache/src/CacheTransactionManagerImpl.cpp
+++ b/cppcache/src/CacheTransactionManagerImpl.cpp
@@ -450,7 +450,7 @@ void 
CacheTransactionManagerImpl::resumeTxUsingTxState(TXState* txState,
         txState->getSuspendedExpiryTaskId());
   } else {
     m_cache->getExpiryTaskManager().resetTask(
-        txState->getSuspendedExpiryTaskId(), std::chrono::seconds(0));
+        txState->getSuspendedExpiryTaskId(), std::chrono::seconds::zero());
   }
 
   // set the current state as the state of the suspended transaction
diff --git a/cppcache/src/EntriesMapFactory.cpp 
b/cppcache/src/EntriesMapFactory.cpp
index 62754c07..2ac150ea 100644
--- a/cppcache/src/EntriesMapFactory.cpp
+++ b/cppcache/src/EntriesMapFactory.cpp
@@ -60,7 +60,8 @@ EntriesMap* EntriesMapFactory::createMap(
     } else {
       return nullptr;
     }
-    if (ttl.count() != 0 || idle.count() != 0) {
+    if (ttl > std::chrono::seconds::zero() ||
+        idle > std::chrono::seconds::zero()) {
       result = new LRUEntriesMap(
           &expiryTaskmanager,
           std::unique_ptr<LRUExpEntryFactory>(
@@ -75,7 +76,8 @@ EntriesMap* EntriesMapFactory::createMap(
           region, lruEvictionAction, lruLimit, concurrencyChecksEnabled,
           concurrency, heapLRUEnabled);
     }
-  } else if (ttl.count() > 0 || idle.count() > 0) {
+  } else if (ttl > std::chrono::seconds::zero() ||
+             idle > std::chrono::seconds::zero()) {
     // create entries with a ExpEntryFactory.
     result = new ConcurrentEntriesMap(
         &expiryTaskmanager,
diff --git a/cppcache/src/EntryExpiryHandler.cpp 
b/cppcache/src/EntryExpiryHandler.cpp
index fcf1b7be..b4d31685 100644
--- a/cppcache/src/EntryExpiryHandler.cpp
+++ b/cppcache/src/EntryExpiryHandler.cpp
@@ -48,7 +48,8 @@ int EntryExpiryHandler::handle_timeout(const ACE_Time_Value& 
current_time,
     auto curr_time = 
std::chrono::system_clock::from_time_t(current_time.sec());
 
     auto lastTimeForExp = expProps.getLastAccessTime();
-    if (m_regionPtr->getAttributes()->getEntryTimeToLive().count() > 0) {
+    if (m_regionPtr->getAttributes()->getEntryTimeToLive() >
+        std::chrono::seconds::zero()) {
       lastTimeForExp = expProps.getLastModifiedTime();
     }
 
diff --git a/cppcache/src/LocalRegion.cpp b/cppcache/src/LocalRegion.cpp
index 9dbf9838..903b062f 100644
--- a/cppcache/src/LocalRegion.cpp
+++ b/cppcache/src/LocalRegion.cpp
@@ -2811,11 +2811,11 @@ ExpirationAction::Action 
LocalRegion::adjustRegionExpiryAction(
   CHECK_DESTROY_PENDING(TryReadGuard, LocalRegion::adjustRegionExpiryAction);
 
  auto attrs = m_regionAttributes;
-  bool hadExpiry = (getRegionExpiryDuration().count() != 0);
-  if (!hadExpiry) {
-    throw IllegalStateException(
-        "Cannot change region ExpirationAction for region created without "
-        "region expiry.");
+ bool hadExpiry = (getRegionExpiryDuration() > std::chrono::seconds::zero());
+ if (!hadExpiry) {
+   throw IllegalStateException(
+       "Cannot change region ExpirationAction for region created without "
+       "region expiry.");
   }
   ExpirationAction::Action oldValue = getRegionExpiryAction();
 
@@ -2831,12 +2831,12 @@ ExpirationAction::Action 
LocalRegion::adjustEntryExpiryAction(
   CHECK_DESTROY_PENDING(TryReadGuard, LocalRegion::adjustEntryExpiryAction);
 
  auto attrs = m_regionAttributes;
-  bool hadExpiry = (getEntryExpiryDuration().count() != 0);
-  if (!hadExpiry) {
-    throw IllegalStateException(
-        "Cannot change entry ExpirationAction for region created without "
-        "entry "
-        "expiry.");
+ bool hadExpiry = (getEntryExpiryDuration() > std::chrono::seconds::zero());
+ if (!hadExpiry) {
+   throw IllegalStateException(
+       "Cannot change entry ExpirationAction for region created without "
+       "entry "
+       "expiry.");
   }
   ExpirationAction::Action oldValue = getEntryExpirationAction();
 
@@ -2850,7 +2850,7 @@ std::chrono::seconds 
LocalRegion::adjustRegionExpiryDuration(
     const std::chrono::seconds& duration) {
   CHECK_DESTROY_PENDING(TryReadGuard, LocalRegion::adjustRegionExpiryDuration);
 
-  bool hadExpiry = (getEntryExpiryDuration().count() != 0);
+  bool hadExpiry = (getEntryExpiryDuration() > std::chrono::seconds::zero());
   if (!hadExpiry) {
     throw IllegalStateException(
         "Cannot change region  expiration duration for region created "
@@ -2869,7 +2869,7 @@ std::chrono::seconds 
LocalRegion::adjustEntryExpiryDuration(
     const std::chrono::seconds& duration) {
   CHECK_DESTROY_PENDING(TryReadGuard, LocalRegion::adjustEntryExpiryDuration);
 
-  bool hadExpiry = (getEntryExpiryDuration().count() != 0);
+  bool hadExpiry = (getEntryExpiryDuration() > std::chrono::seconds::zero());
   if (!hadExpiry) {
     throw IllegalStateException(
         "Cannot change entry expiration duration for region created without "
@@ -2922,7 +2922,7 @@ bool LocalRegion::isEntryIdletimeEnabled() {
 }
 
 ExpirationAction::Action LocalRegion::getEntryExpirationAction() const {
-  if (m_regionAttributes->getEntryTimeToLive().count() > 0) {
+  if (m_regionAttributes->getEntryTimeToLive() > std::chrono::seconds::zero()) 
{
     return m_regionAttributes->getEntryTimeToLiveAction();
   } else {
     return m_regionAttributes->getEntryIdleTimeoutAction();
diff --git a/cppcache/src/PoolFactory.cpp b/cppcache/src/PoolFactory.cpp
index 3096b076..882624ee 100644
--- a/cppcache/src/PoolFactory.cpp
+++ b/cppcache/src/PoolFactory.cpp
@@ -52,7 +52,7 @@ const std::chrono::milliseconds
     PoolFactory::DEFAULT_UPDATE_LOCATOR_LIST_INTERVAL = 
std::chrono::seconds{5};
 
 const std::chrono::milliseconds PoolFactory::DEFAULT_STATISTIC_INTERVAL =
-    std::chrono::milliseconds{-1};
+    std::chrono::milliseconds::zero();
 
 const std::chrono::milliseconds
     PoolFactory::DEFAULT_SUBSCRIPTION_MESSAGE_TRACKING_TIMEOUT =
@@ -73,9 +73,8 @@ PoolFactory::~PoolFactory() {}
 
 PoolFactory& PoolFactory::setFreeConnectionTimeout(
     std::chrono::milliseconds connectionTimeout) {
-  // TODO GEODE-3136 - Is this true?
   if (connectionTimeout <= std::chrono::milliseconds::zero()) {
-    throw std::invalid_argument("connectionTimeout must greater than 0.");
+    throw IllegalArgumentException("connectionTimeout must greater than 0.");
   }
 
   m_attrs->setFreeConnectionTimeout(connectionTimeout);
@@ -84,10 +83,9 @@ PoolFactory& PoolFactory::setFreeConnectionTimeout(
 
 PoolFactory& PoolFactory::setLoadConditioningInterval(
     std::chrono::milliseconds loadConditioningInterval) {
-  // TODO GEODE-3136 - Is this true?
-  if (loadConditioningInterval <= std::chrono::milliseconds::zero()) {
-    throw std::invalid_argument(
-        "loadConditioningInterval must greater than 0.");
+  if (loadConditioningInterval < std::chrono::milliseconds::zero()) {
+    throw IllegalArgumentException(
+        "loadConditioningInterval must greater than or equlal to 0.");
   }
 
   m_attrs->setLoadConditioningInterval(loadConditioningInterval);
@@ -99,15 +97,15 @@ PoolFactory& PoolFactory::setSocketBufferSize(int 
bufferSize) {
   return *this;
 }
 
-PoolFactory& PoolFactory::setThreadLocalConnections(bool 
threadLocalConnections) {
+PoolFactory& PoolFactory::setThreadLocalConnections(
+    bool threadLocalConnections) {
   m_attrs->setThreadLocalConnectionSetting(threadLocalConnections);
   return *this;
 }
 
 PoolFactory& PoolFactory::setReadTimeout(std::chrono::milliseconds timeout) {
-  // TODO GEODE-3136 - Is this true?
   if (timeout <= std::chrono::milliseconds::zero()) {
-    throw std::invalid_argument("timeout must greater than 0.");
+    throw IllegalArgumentException("timeout must greater than 0.");
   }
 
   m_attrs->setReadTimeout(timeout);
@@ -124,7 +122,13 @@ PoolFactory& PoolFactory::setMaxConnections(int 
maxConnections) {
   return *this;
 }
 
-PoolFactory& PoolFactory::setIdleTimeout(std::chrono::milliseconds 
idleTimeout) {
+PoolFactory& PoolFactory::setIdleTimeout(
+    std::chrono::milliseconds idleTimeout) {
+  if (idleTimeout < std::chrono::milliseconds::zero()) {
+    throw IllegalArgumentException(
+        "idleTimeout must greater than or equlal to 0.");
+  }
+
   m_attrs->setIdleTimeout(idleTimeout);
   return *this;
 }
@@ -134,10 +138,10 @@ PoolFactory& PoolFactory::setRetryAttempts(int 
retryAttempts) {
   return *this;
 }
 
-PoolFactory& PoolFactory::setPingInterval(std::chrono::milliseconds 
pingInterval) {
-  // TODO GEODE-3136 - Is this true?
+PoolFactory& PoolFactory::setPingInterval(
+    std::chrono::milliseconds pingInterval) {
   if (pingInterval <= std::chrono::milliseconds::zero()) {
-    throw std::invalid_argument("timeout must greater than 0.");
+    throw IllegalArgumentException("timeout must greater than 0.");
   }
 
   m_attrs->setPingInterval(pingInterval);
@@ -146,9 +150,8 @@ PoolFactory& 
PoolFactory::setPingInterval(std::chrono::milliseconds pingInterval
 
 PoolFactory& PoolFactory::setUpdateLocatorListInterval(
     const std::chrono::milliseconds updateLocatorListInterval) {
-  // TODO GEODE-3136 - Is this true?
   if (updateLocatorListInterval < std::chrono::milliseconds::zero()) {
-    throw std::invalid_argument("timeout must be positive.");
+    throw IllegalArgumentException("timeout must be positive.");
   }
 
   m_attrs->setUpdateLocatorListInterval(updateLocatorListInterval);
@@ -157,9 +160,8 @@ PoolFactory& PoolFactory::setUpdateLocatorListInterval(
 
 PoolFactory& PoolFactory::setStatisticInterval(
     std::chrono::milliseconds statisticInterval) {
-  // TODO GEODE-3136 - Consider 0 to disable
-  if (statisticInterval.count() <= -1) {
-    throw std::invalid_argument("timeout must greater than -1.");
+  if (statisticInterval < std::chrono::milliseconds::zero()) {
+    throw IllegalArgumentException("timeout must greater than or equal to 0.");
   }
 
   m_attrs->setStatisticInterval(statisticInterval);
@@ -198,9 +200,8 @@ PoolFactory& PoolFactory::setSubscriptionRedundancy(int 
redundancy) {
 
 PoolFactory& PoolFactory::setSubscriptionMessageTrackingTimeout(
     std::chrono::milliseconds messageTrackingTimeout) {
-  // TODO GEODE-3136 - Is this true?
   if (messageTrackingTimeout <= std::chrono::milliseconds::zero()) {
-    throw std::invalid_argument("timeout must greater than 0.");
+    throw IllegalArgumentException("timeout must greater than 0.");
   }
 
   m_attrs->setSubscriptionMessageTrackingTimeout(messageTrackingTimeout);
@@ -209,16 +210,16 @@ PoolFactory& 
PoolFactory::setSubscriptionMessageTrackingTimeout(
 
 PoolFactory& PoolFactory::setSubscriptionAckInterval(
     std::chrono::milliseconds ackInterval) {
-  // TODO GEODE-3136 - Is this true?
   if (ackInterval <= std::chrono::milliseconds::zero()) {
-    throw std::invalid_argument("timeout must greater than 0.");
+    throw IllegalArgumentException("timeout must greater than 0.");
   }
 
   m_attrs->setSubscriptionAckInterval(ackInterval);
   return *this;
 }
 
-PoolFactory& PoolFactory::setMultiuserAuthentication(bool 
multiuserAuthentication) {
+PoolFactory& PoolFactory::setMultiuserAuthentication(
+    bool multiuserAuthentication) {
   m_attrs->setMultiuserSecureModeEnabled(multiuserAuthentication);
   return *this;
 }
@@ -241,7 +242,7 @@ std::shared_ptr<Pool> PoolFactory::create(std::string name) 
{
     // Create a clone of Attr;
     auto copyAttrs = m_attrs->clone();
 
-    CacheImpl* cacheImpl = CacheRegionHelper::getCacheImpl(&m_cache);
+    auto cacheImpl = CacheRegionHelper::getCacheImpl(&m_cache);
 
     if (m_cache.isClosed()) {
       throw CacheClosedException("Cache is closed");
@@ -268,8 +269,7 @@ std::shared_ptr<Pool> PoolFactory::create(std::string name) 
{
     }
     if (!copyAttrs->getSubscriptionEnabled() &&
         copyAttrs->getSubscriptionRedundancy() == 0 && !tccm.isDurable()) {
-      if (copyAttrs
-              ->getThreadLocalConnectionSetting() /*&& 
!copyAttrs->getPRSingleHopEnabled()*/) {
+      if (copyAttrs->getThreadLocalConnectionSetting()) {
         // TODO: what should we do for sticky connections
         poolDM = std::make_shared<ThinClientPoolStickyDM>(name.c_str(),
                                                           copyAttrs, tccm);
@@ -280,8 +280,7 @@ std::shared_ptr<Pool> PoolFactory::create(std::string name) 
{
       }
     } else {
       LOGDEBUG("ThinClientPoolHADM created ");
-      if (copyAttrs
-              ->getThreadLocalConnectionSetting() /*&& 
!copyAttrs->getPRSingleHopEnabled()*/) {
+      if (copyAttrs->getThreadLocalConnectionSetting()) {
         poolDM = std::make_shared<ThinClientPoolStickyHADM>(name.c_str(),
                                                             copyAttrs, tccm);
       } else {
diff --git a/cppcache/src/SystemProperties.cpp 
b/cppcache/src/SystemProperties.cpp
index 52a82f3a..8ae4e5c6 100644
--- a/cppcache/src/SystemProperties.cpp
+++ b/cppcache/src/SystemProperties.cpp
@@ -91,8 +91,8 @@ const char DefaultDurableClientId[] = "";
 constexpr auto DefaultDurableTimeout = std::chrono::seconds(300);
 
 constexpr auto DefaultConnectTimeout = std::chrono::seconds(59);
-constexpr auto DefaultConnectWaitTimeout = std::chrono::seconds(0);
-constexpr auto DefaultBucketWaitTimeout = std::chrono::seconds(0);
+constexpr auto DefaultConnectWaitTimeout = std::chrono::seconds::zero();
+constexpr auto DefaultBucketWaitTimeout = std::chrono::seconds::zero();
 
 constexpr auto DefaultSamplingInterval = std::chrono::seconds(1);
 const bool DefaultSamplingEnabled = true;
diff --git a/cppcache/src/TcrConnection.cpp b/cppcache/src/TcrConnection.cpp
index 6c653e17..d892f5a8 100644
--- a/cppcache/src/TcrConnection.cpp
+++ b/cppcache/src/TcrConnection.cpp
@@ -1407,7 +1407,7 @@ std::shared_ptr<CacheableString> 
TcrConnection::readHandshakeString(
   }
 }
 bool TcrConnection::hasExpired(const std::chrono::milliseconds& expiryTime) {
-  if (expiryTime.count() < 0) {
+  if (expiryTime <= std::chrono::milliseconds::zero()) {
     return false;
   }
 
@@ -1421,7 +1421,7 @@ bool TcrConnection::hasExpired(const 
std::chrono::milliseconds& expiryTime) {
 }
 
 bool TcrConnection::isIdle(const std::chrono::milliseconds& idleTime) {
-  if (idleTime.count() < 0) {
+  if (idleTime <= std::chrono::milliseconds::zero()) {
     return false;
   }
 
diff --git a/cppcache/src/ThinClientPoolDM.cpp 
b/cppcache/src/ThinClientPoolDM.cpp
index d2c9a857..0b25b900 100644
--- a/cppcache/src/ThinClientPoolDM.cpp
+++ b/cppcache/src/ThinClientPoolDM.cpp
@@ -328,14 +328,13 @@ void ThinClientPoolDM::startBackgroundThreads() {
   auto idle = getIdleTimeout();
   auto load = getLoadConditioningInterval();
 
-  // TODO GEODE-3136 - consider using 0 rather than -1.
-  if (load.count() >= 0) {
-    if (load < idle || idle.count() < 0) {
+  if (load > std::chrono::milliseconds::zero()) {
+    if (load < idle || idle <= std::chrono::milliseconds::zero()) {
       idle = load;
     }
   }
 
-  if (idle.count() != -1) {
+  if (idle > std::chrono::milliseconds::zero()) {
     LOGDEBUG(
         "ThinClientPoolDM::startBackgroundThreads: Starting manageConnections "
         "task");
@@ -360,11 +359,11 @@ void ThinClientPoolDM::startBackgroundThreads() {
 
   LOGDEBUG(
       "ThinClientPoolDM::startBackgroundThreads: Starting pool stat sampler");
-  if (m_PoolStatsSampler == nullptr && getStatisticInterval().count() > -1 &&
+  if (m_PoolStatsSampler == nullptr &&
+      getStatisticInterval() > std::chrono::milliseconds::zero() &&
       props.statisticsEnabled()) {
-    m_PoolStatsSampler =
-        new PoolStatsSampler(getStatisticInterval().count() / 1000 + 1,
-                             m_connManager.getCacheImpl(), this);
+    m_PoolStatsSampler = new PoolStatsSampler(
+        getStatisticInterval(), m_connManager.getCacheImpl(), this);
     m_PoolStatsSampler->start();
   }
 
@@ -2176,9 +2175,8 @@ void 
ThinClientPoolDM::setThreadLocalConnection(TcrConnection* conn) {
   m_manager->addStickyConnection(conn);
 }
 
-bool ThinClientPoolDM::hasExpired(TcrConnection* conn) {
-  const auto& load = getLoadConditioningInterval();
-  return conn->hasExpired(load);
+inline bool ThinClientPoolDM::hasExpired(TcrConnection* conn) {
+  return conn->hasExpired(getLoadConditioningInterval());
 }
 
 bool ThinClientPoolDM::canItBeDeleted(TcrConnection* conn) {
@@ -2186,9 +2184,8 @@ bool ThinClientPoolDM::canItBeDeleted(TcrConnection* 
conn) {
   auto idle = getIdleTimeout();
   int min = getMinConnections();
 
-  // TODO GEODE-3136 reconsider use of -1
-  if (load.count() >= 0) {
-    if (load < idle || idle.count() < 0) {
+  if (load > std::chrono::milliseconds::zero()) {
+    if (load < idle || idle <= std::chrono::milliseconds::zero()) {
       idle = load;
     }
   }
diff --git a/cppcache/src/TombstoneList.cpp b/cppcache/src/TombstoneList.cpp
index 582ef898..f4437a29 100644
--- a/cppcache/src/TombstoneList.cpp
+++ b/cppcache/src/TombstoneList.cpp
@@ -48,7 +48,7 @@ long TombstoneList::getExpiryTask(TombstoneExpiryHandler** 
handler) {
                                         m_cacheImpl);
   tombstoneEntryPtr->setHandler(*handler);
   long id = m_cacheImpl->getExpiryTaskManager().scheduleExpiryTask(
-      *handler, duration, std::chrono::seconds(0));
+      *handler, duration, std::chrono::seconds::zero());
   return id;
 }
 
diff --git a/cppcache/src/statistics/PoolStatsSampler.cpp 
b/cppcache/src/statistics/PoolStatsSampler.cpp
index ea062791..d75b4809 100644
--- a/cppcache/src/statistics/PoolStatsSampler.cpp
+++ b/cppcache/src/statistics/PoolStatsSampler.cpp
@@ -38,7 +38,7 @@ using std::chrono::nanoseconds;
 
 const char* PoolStatsSampler::NC_PSS_Thread = "NC PSS Thread";
 
-PoolStatsSampler::PoolStatsSampler(int64_t sampleRate, CacheImpl* cache,
+PoolStatsSampler::PoolStatsSampler(milliseconds sampleRate, CacheImpl* cache,
                                    ThinClientPoolDM* distMan)
     : m_sampleRate(sampleRate),
       m_distMan(distMan),
@@ -57,14 +57,13 @@ PoolStatsSampler::~PoolStatsSampler() {
 int32_t PoolStatsSampler::svc() {
   DistributedSystemImpl::setThreadName(NC_PSS_Thread);
   auto msSpentWorking = milliseconds::zero();
-  auto samplingRate = milliseconds(m_sampleRate);
   // ACE_Guard < ACE_Recursive_Thread_Mutex > _guard( m_lock );
   while (!m_stopRequested) {
     auto sampleStart = high_resolution_clock::now();
     putStatsInAdminRegion();
     nanoseconds spentWorking = high_resolution_clock::now() - sampleStart;
     auto sleepDuration =
-        samplingRate - duration_cast<milliseconds>(spentWorking);
+        m_sampleRate - duration_cast<milliseconds>(spentWorking);
     static const auto wakeInterval = milliseconds(100);
     while (!m_stopRequested && sleepDuration > milliseconds::zero()) {
       std::this_thread::sleep_for(sleepDuration > wakeInterval ? wakeInterval
diff --git a/cppcache/src/statistics/PoolStatsSampler.hpp 
b/cppcache/src/statistics/PoolStatsSampler.hpp
index 67932e12..8ce50ed9 100644
--- a/cppcache/src/statistics/PoolStatsSampler.hpp
+++ b/cppcache/src/statistics/PoolStatsSampler.hpp
@@ -45,7 +45,7 @@ using client::AdminRegion;
 class StatisticsManager;
 class _GEODE_EXPORT PoolStatsSampler : public ACE_Task_Base {
  public:
-  PoolStatsSampler(int64_t sampleRate, CacheImpl* cache,
+  PoolStatsSampler(std::chrono::milliseconds sampleRate, CacheImpl* cache,
                    ThinClientPoolDM* distMan);
   void start();
   void stop();
@@ -60,7 +60,7 @@ class _GEODE_EXPORT PoolStatsSampler : public ACE_Task_Base {
   void putStatsInAdminRegion();
   volatile bool m_running;
   volatile bool m_stopRequested;
-  int64_t m_sampleRate;
+  std::chrono::milliseconds m_sampleRate;
   std::shared_ptr<AdminRegion> m_adminRegion;
   ThinClientPoolDM* m_distMan;
   ACE_Recursive_Thread_Mutex m_lock;


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> Standardize on model for infinity that doesn't use int overloading
> ------------------------------------------------------------------
>
>                 Key: GEODE-3570
>                 URL: https://issues.apache.org/jira/browse/GEODE-3570
>             Project: Geode
>          Issue Type: Improvement
>          Components: native client
>            Reporter: Mark Hanson
>            Assignee: Jacob S. Barrett
>            Priority: Major
>              Labels: pull-request-available
>
> See discussion here http://markmail.org/thread/hwrkg5cra57lyvo3
> Currently the code in the native client follows various conventions. We would 
> like to standardize the code to use a model where instead of overloading int 
> to mean infinity on a wait, to create two separate function calls. One for 
> wait indefinitely wait()  and one to wait for a duration 
> waitDuration(duration)
> ThinClientRegion.cpp:2996 executeFunction needs refactoring.
> PoolAttributes.hpp:69 setRetryAttempts needs to be retitled attempts and then 
> the using functions need to use it appropriately.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to