Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IceGrid Glacier2 session managers to keep sessions alive #2756

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

pepone
Copy link
Member

@pepone pepone commented Sep 20, 2024

This PR fix the Glacier2 session managers implemented in IceGrid to correctly keep sessions alive.

Copy link
Member

@bernardnormier bernardnormier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@@ -73,6 +73,8 @@ class SessionI : public BaseSessionI, public Session

Ice::ObjectPrx _register(const SessionServantManagerPtr&, const Ice::ConnectionPtr&);

void ice_ping(const Ice::Current&) const;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add virtual for consistency. "virtual" stands for override.

Copy link
Member

@bentoi bentoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear why this is needed. Keep alive was deprecated in favor of ACM heartbeats and afaict this PR has nothing to do with Glacier2 session managers.

@pepone
Copy link
Member Author

pepone commented Sep 25, 2024

It's not clear why this is needed

IceGrid implements two Glacier2 session managers documented in https://doc.zeroc.com/ice/latest/ice-services/icegrid/glacier2-integration-with-icegrid#Glacier2IntegrationwithIceGrid-ResourceAllocationusingGlacier2andIceGrid

Glacier2 router, send ice_ping messages to sessions to ensure they are keep alive, this is done from the Heartbeat callback

_sessionRouter->refreshSession(connection);

//
// Ping the session to ensure it does not timeout.
//
session->begin_ice_ping(Ice::newCallback(
new PingCallback(this, callback, current.con), &PingCallback::finished));

This doesn't work for IceGrid/SessionManager or IceGrid/AdminSSLSessionManager which are only keep alive by calling keepAlive.

Glacier2 sessions are added to IceGrid reaper without a connection, which means there is no ACM callback added to them.

_reaper->add(new SessionReapable<SessionI>(_database->getTraceLevels()->logger, session), timeout);

@pepone pepone requested a review from bentoi September 26, 2024 07:48
@pepone pepone merged commit c2e07e9 into zeroc-ice:3.7 Sep 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants