zeroc-ice / ice

All-in-one solution for creating networked applications with RPC, pub/sub, server deployment, and more.
https://zeroc.com
GNU General Public License v2.0
2k stars 592 forks source link

Simplify ACM #1505

Open bernardnormier opened 9 months ago

bernardnormier commented 9 months ago

ACM in Ice 3.7 and earlier releases is too complicated, with too many options to compose.

We implemented a much simpler ACM (not called ACM) in IceRPC for the ice protocol, see:

https://github.com/icerpc/icerpc-csharp/blob/89a7a8466c37b2d1bcf2f8c7cd593cbf7df14757/src/IceRpc/ConnectionOptions.cs#L27

We should port this design to Ice.

In particular, IceRPC makes a distinction between idle and inactive, and we should do the same in Ice.

See https://github.com/icerpc/icerpc-csharp/blob/89a7a8466c37b2d1bcf2f8c7cd593cbf7df14757/src/IceRpc/ConnectionOptions.cs#L35 https://github.com/icerpc/icerpc-csharp/blob/89a7a8466c37b2d1bcf2f8c7cd593cbf7df14757/src/IceRpc/ConnectionOptions.cs#L46

bernardnormier commented 9 months ago

Proposal

  1. Remove all Ice.ACM and ObjectAdapter.ACM properties and replace them with:
    Ice.IdleTimeout  # corresponds to IceIdleTimeout in IceRPC, with same default (60 seconds)
    Ice.InactivityTimeout # corresponds to InactivityTimeout in IceRPC, with same default (5 * 60 seconds)
    Ice.EnableIdleCheck # corresponds to EnableIdleCheck in IceRPC, with same default (false) for interop with Ice 3.7

These properties apply to all connections (client and server). You can override these properties for a specific object adapter with adapter.IdleTimeout etc.

  1. Update local interface Connection as follows. See https://doc.zeroc.com/ice/3.7/client-server-features/connection-management/using-connections
    local interface Connection
    {
       ... this proposal changes only ACM related operations ...
        void setCloseCallback(CloseCallback callback); // no change
        void setHeartbeatCallback(HeartbeatCallback callback); // no change

        // Remove setACM / getACM and timeout

        // New operations; all the timeouts are in seconds.
        int getIdleTimeout();
        void setIdleTimeout(int timeout);

        int getInactivityTimeout();
        void setInactivityTimeout(int timeout);

        bool isIdleCheckEnabled();
        void disableIdleCheck();
        void enableIdleCheck();    
    }
  1. Change the logic to be like IceRPC, in particular:
    • send pings every idle timeout / 2 unless there is some other write activity
    • local write activity (including pings) does not prevent the connection from becoming idle locally (idle => abort connection when EnableIdleCheck is true)
externl commented 3 months ago

Ice.IdleTimeout # corresponds to IceIdleTimeout in IceRPC, with same default (60 seconds)

According to https://docs.icerpc.dev/icerpc/slic-transport/connection-idle-timeout

The default idle timeout is 30 seconds.

EDIT: Never mind, I was looking at SLIC.

bernardnormier commented 3 months ago

The relevant page in IceRPC C# is: https://docs.icerpc.dev/api/csharp/api/IceRpc.ConnectionOptions.html#IceRpc_ConnectionOptions_IceIdleTimeout