[GitHub] curator pull request: [CURATOR-247] Extend Curator's connection st...

[cammckenzie <gi...@git.apache.org>]

Github user cammckenzie commented on a diff in the pull request:

    https://github.com/apache/curator/pull/97#discussion_r38377860
  
    --- Diff: curator-client/src/main/java/org/apache/curator/connection/ConnectionHandlingPolicy.java ---
    @@ -0,0 +1,72 @@
    +package org.apache.curator.connection;
    +
    +import org.apache.curator.CuratorZookeeperClient;
    +import org.apache.curator.RetryLoop;
    +import java.util.concurrent.Callable;
    +
    +/**
    + * Abstracts connection handling so that Curator can emulate it's old, pre 3.0.0
    + * handling and update to newer handling.
    + */
    +public interface ConnectionHandlingPolicy
    +{
    +    /**
    +     * Return true if this policy should behave like the pre-3.0.0 version of Curator
    +     *
    +     * @return true/false
    +     */
    +    boolean isEmulatingClassicHandling();
    +
    +    /**
    +     * Called by {@link RetryLoop#callWithRetry(CuratorZookeeperClient, Callable)} to do the work
    +     * of retrying
    +     *
    +     * @param client client
    +     * @param proc the procedure to retry
    +     * @return result
    +     * @throws Exception errors
    +     */
    +    <T> T callWithRetry(CuratorZookeeperClient client, Callable<T> proc) throws Exception;
    +
    +    enum CheckTimeoutsResult
    +    {
    +        /**
    +         * Do nothing
    +         */
    +        NOP,
    +
    +        /**
    +         * handle a new connection string
    +         */
    +        NEW_CONNECTION_STRING,
    +
    +        /**
    +         * reset/recreate the internal ZooKeeper connection
    +         */
    +        RESET_CONNECTION,
    +
    +        /**
    +         * handle a connection timeout
    +         */
    +        CONNECTION_TIMEOUT,
    +
    +        /**
    +         * handle a session timeout
    +         */
    +        SESSION_TIMEOUT
    +    }
    +
    +    /**
    +     * Check timeouts. NOTE: this method is only called when an attempt to access to the ZooKeeper instances
    +     * is made and the connection has not completed.
    +     *
    +     * @param hasNewConnectionString proc to call to check if there is a new connection string. Important: the internal state is cleared after
    +     *                               this is called so you MUST handle the new connection string if <tt>true</tt> is returned
    +     * @param connectionStartMs the epoch/ms time that the connection was first initiated
    +     * @param sessionTimeoutMs the configured session timeout in milliseconds
    --- End diff --
    
    Should this be the negotiated session timeout rather than the configured session timeout? Looking at the code that calls into this interface, it would seem that the configured timeout from the build is being used. Is that correct though? I would have thought that the negotiated timeout was what was important as it's the source of truth.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---