Revert "Remove IceGrid SessionTimeout properties (#2076)" (#2953)

zeroc-ice · Oct 22, 2024 · ffaef5e · ffaef5e
1 parent 6e02487
commit ffaef5e
Show file tree

Hide file tree

Showing 12 changed files with 47 additions and 14 deletions.
diff --git a/config/PropertyNames.xml b/config/PropertyNames.xml
@@ -278,11 +278,14 @@
         <property name="Registry.DynamicRegistration" languages="cpp" />
         <property name="Registry.Internal" class="ObjectAdapter" languages="cpp" />
         <property name="Registry.LMDB" class="LMDB" languages="cpp"/>
+        <property name="Registry.NodeSessionTimeout" languages="cpp" default="30" />
         <property name="Registry.PermissionsVerifier" class="Proxy" languages="cpp" />
         <property name="Registry.ReplicaName" languages="cpp" default="Master" />
+        <property name="Registry.ReplicaSessionTimeout" languages="cpp" default="30" />
         <property name="Registry.Server" class="ObjectAdapter" languages="cpp" />
         <property name="Registry.SessionFilters" languages="cpp" default="0" />
         <property name="Registry.SessionManager" class="ObjectAdapter" languages="cpp" />
+        <property name="Registry.SessionTimeout" languages="cpp" default="30" />
         <property name="Registry.SSLPermissionsVerifier" class="Proxy" languages="cpp"/>
         <property name="Registry.Trace.Admin" languages="cpp" default="0" />
         <property name="Registry.Trace.Application" languages="cpp" default="0" />

diff --git a/cpp/src/Ice/PropertyNames.cpp b/cpp/src/Ice/PropertyNames.cpp
@@ -374,11 +374,14 @@ const Property IceGridPropsData[] =
     Property{"Registry.DynamicRegistration", "", false, false, nullptr},
     Property{"Registry.Internal", "", false, false, &PropertyNames::ObjectAdapterProps},
     Property{"Registry.LMDB", "", false, false, &PropertyNames::LMDBProps},
+    Property{"Registry.NodeSessionTimeout", "30", false, false, nullptr},
     Property{"Registry.PermissionsVerifier", "", false, false, &PropertyNames::ProxyProps},
     Property{"Registry.ReplicaName", "Master", false, false, nullptr},
+    Property{"Registry.ReplicaSessionTimeout", "30", false, false, nullptr},
     Property{"Registry.Server", "", false, false, &PropertyNames::ObjectAdapterProps},
     Property{"Registry.SessionFilters", "0", false, false, nullptr},
     Property{"Registry.SessionManager", "", false, false, &PropertyNames::ObjectAdapterProps},
+    Property{"Registry.SessionTimeout", "30", false, false, nullptr},
     Property{"Registry.SSLPermissionsVerifier", "", false, false, &PropertyNames::ProxyProps},
     Property{"Registry.Trace.Admin", "0", false, false, nullptr},
     Property{"Registry.Trace.Application", "0", false, false, nullptr},
@@ -402,7 +405,7 @@ const PropertyArray PropertyNames::IceGridProps
     "IceGrid",
     false,
     IceGridPropsData,
-    60
+    63
 };
 
 const Property IceSSLPropsData[] =

diff --git a/cpp/src/IceGrid/AdminSessionI.h b/cpp/src/IceGrid/AdminSessionI.h
@@ -22,8 +22,7 @@ namespace IceGrid
 
         Ice::ObjectPrx _register(const std::shared_ptr<SessionServantManager>&, const Ice::ConnectionPtr&);
 
-        // keepAlive is deprecated and kept only for compatibility with old clients. It does nothing now.
-        void keepAlive(const Ice::Current&) override {}
+        void keepAlive(const Ice::Current& current) override { BaseSessionI::keepAlive(current); }
 
         std::optional<AdminPrx> getAdmin(const Ice::Current&) const override;
         std::optional<Ice::ObjectPrx> getAdminCallbackTemplate(const Ice::Current&) const override;

diff --git a/cpp/src/IceGrid/InternalRegistryI.cpp b/cpp/src/IceGrid/InternalRegistryI.cpp
@@ -35,9 +35,8 @@ InternalRegistryI::InternalRegistryI(
       _session(session)
 {
     auto properties = database->getCommunicator()->getProperties();
-    // TODO: temporary. For now, synchronized with the default idle timeout.
-    _nodeSessionTimeout = chrono::seconds(60);
-    _replicaSessionTimeout = chrono::seconds(60);
+    _nodeSessionTimeout = chrono::seconds(properties->getIcePropertyAsInt("IceGrid.Registry.NodeSessionTimeout"));
+    _replicaSessionTimeout = chrono::seconds(properties->getIcePropertyAsInt("IceGrid.Registry.ReplicaSessionTimeout"));
 }
 
 optional<NodeSessionPrx>

diff --git a/cpp/src/IceGrid/RegistryI.cpp b/cpp/src/IceGrid/RegistryI.cpp
@@ -250,8 +250,7 @@ RegistryI::startImpl()
     _replicaName = properties->getIceProperty("IceGrid.Registry.ReplicaName");
     _master = _replicaName == "Master";
 
-    // TODO: temporary. For now, synchronized with the default idle timeout.
-    _sessionTimeout = chrono::seconds(60);
+    _sessionTimeout = chrono::seconds(properties->getIcePropertyAsInt("IceGrid.Registry.SessionTimeout"));
 
     if (!_initFromReplica.empty() && (_initFromReplica == _replicaName || (_master && _initFromReplica == "Master")))
     {

diff --git a/cpp/src/IceGrid/SessionI.cpp b/cpp/src/IceGrid/SessionI.cpp
@@ -63,6 +63,24 @@ BaseSessionI::BaseSessionI(const string& id, const string& prefix, const shared_
     }
 }
 
+void
+BaseSessionI::keepAlive(const Ice::Current&)
+{
+    lock_guard lock(_mutex);
+    if (_destroyed)
+    {
+        throw Ice::ObjectNotExistException{__FILE__, __LINE__};
+    }
+
+    _timestamp = chrono::steady_clock::now();
+
+    if (_traceLevels->session > 1)
+    {
+        Ice::Trace out(_traceLevels->logger, _traceLevels->sessionCat);
+        out << _prefix << " session '" << _id << "' keep alive";
+    }
+}
+
 void
 BaseSessionI::destroyImpl(bool)
 {

diff --git a/cpp/src/IceGrid/SessionI.h b/cpp/src/IceGrid/SessionI.h
@@ -24,6 +24,8 @@ namespace IceGrid
     public:
         virtual ~BaseSessionI() = default;
 
+        virtual void keepAlive(const Ice::Current&);
+
         std::chrono::steady_clock::time_point timestamp() const;
         void shutdown();
         std::optional<Glacier2::IdentitySetPrx> getGlacier2IdentitySet();
@@ -57,8 +59,7 @@ namespace IceGrid
 
         Ice::ObjectPrx _register(const std::shared_ptr<SessionServantManager>&, const Ice::ConnectionPtr&);
 
-        // keepAlive is deprecated and kept only for compatibility with old clients. It does nothing now.
-        void keepAlive(const Ice::Current&) final {}
+        void keepAlive(const Ice::Current& current) final { BaseSessionI::keepAlive(current); }
         void allocateObjectByIdAsync(
             Ice::Identity id,
             std::function<void(const std::optional<Ice::ObjectPrx>& returnValue)> response,

diff --git a/cpp/test/IceGrid/replication/application.xml b/cpp/test/IceGrid/replication/application.xml
@@ -45,6 +45,7 @@
         <property name="IceGrid.Registry.PermissionsVerifier" value="RepTestIceGrid/NullPermissionsVerifier"/>
         <property name="IceGrid.Registry.SSLPermissionsVerifier" value="RepTestIceGrid/NullSSLPermissionsVerifier"/>
         <property name="IceGrid.Registry.AdminPermissionsVerifier" value="RepTestIceGrid/NullPermissionsVerifier"/>
+        <property name="IceGrid.Registry.SessionTimeout" value="0"/>
         <property name="IceGrid.Registry.DynamicRegistration" value="1"/>
         <property name="Ice.Default.Locator" value="RepTestIceGrid/Locator:default -p 12050:default -p 12051:default -p 12052"/>
         <property name="IceGrid.Registry.Trace.Replica" value="1"/>

diff --git a/java/src/IceGridGUI/src/main/java/com/zeroc/IceGridGUI/SessionKeeper.java b/java/src/IceGridGUI/src/main/java/com/zeroc/IceGridGUI/SessionKeeper.java
@@ -247,6 +247,11 @@ com.zeroc.Ice.Object removeCallback(String name, String facet) {
         }
 
         void close(boolean destroySession) {
+            if (_keepAliveFuture != null) {
+                _keepAliveFuture.cancel(false);
+                _keepAliveFuture = null;
+            }
+
             if (_adapter != null) {
                 _adapter.destroy();
                 _adapter = null;
@@ -371,6 +376,8 @@ public void registerObservers() throws Throwable {
         private final AdminSessionPrx _session;
         private final boolean _routed;
 
+        private java.util.concurrent.Future<?> _keepAliveFuture;
+
         private com.zeroc.Ice.ObjectAdapter _adapter;
         private AdminPrx _admin;
         private String _serverAdminCategory;

diff --git a/scripts/IceGridUtil.py b/scripts/IceGridUtil.py
@@ -202,6 +202,7 @@ def getProps(self, current):
             "IceGrid.Registry.Discovery.Port": current.driver.getTestPort(99),
             "IceGrid.Registry.SessionManager.Endpoints": "default",
             "IceGrid.Registry.AdminSessionManager.Endpoints": "default",
+            "IceGrid.Registry.SessionTimeout": 60,
             "IceGrid.Registry.ReplicaName": self.name,
             "Ice.ProgramName": self.name,
             "Ice.PrintAdapterReady": 1,

diff --git a/slice/IceGrid/Admin.ice b/slice/IceGrid/Admin.ice
@@ -752,8 +752,9 @@ module IceGrid
     /// @see Registry
     interface AdminSession extends Glacier2::Session
     {
-        /// Keep the session alive. This operation is provided for backwards compatibility. As of Ice 3.8, there is no
-        /// need to call this operation and its implementation does nothing.
+        /// Keep the session alive. Clients should call this operation regularly to prevent the server from reaping the
+        /// session.
+        /// @see Registry#getSessionTimeout
         idempotent void keepAlive();
 
         /// Get the admin interface. The admin object returned by this operation can only be accessed by the session.

diff --git a/slice/IceGrid/Session.ice b/slice/IceGrid/Session.ice
@@ -25,8 +25,9 @@ module IceGrid
     /// @see Registry
     interface Session extends Glacier2::Session
     {
-        /// Keep the session alive. This operation is provided for backwards compatibility. As of Ice 3.8, there is no
-        /// need to call this operation and its implementation does nothing.
+        /// Keep the session alive. Clients should call this operation regularly to prevent the server from reaping the
+        /// session.
+        /// @see Registry#getSessionTimeout
         idempotent void keepAlive();
 
         /// Allocate an object. Depending on the allocation timeout, this operation might hang until the object is