[enh] Configurable connect timeout in TCPConnectorHandler.

[enh] Configurable connect timeout in TCPConnectorHandler.

Hey guys,

I was thinking about a small enhancement in the code of the
TCPConnectorHandler. It would be useful from a user perspective to have a
connect timeout configurable and now it's fixed to 30 seconds. Only a
smooth change is required to enable this feature. What do you think about
it? Btw what are the steps for joining active development of the framework?

Re: [enh] Configurable connect timeout in TCPConnectorHandler.

Hi Piotr,

Piotr Bazan wrote:
> Hey guys,
>
> I was thinking about a small enhancement in the code of the
> TCPConnectorHandler. It would be useful from a user perspective to have a
> connect timeout configurable and now it's fixed to 30 seconds. Only a
> smooth change is required to enable this feature.

Right now the value is defined by invoking
DefaultSelectionKeyHandler.setTimeout(). Are you proposing to configure
that value from the TCPConnectorHandler? I agree the current
implementation is not intuitive :-)

What do you think about
> it? Btw what are the steps for joining active development of the framework?

Just submit patches (svn diff) to dev@grizzly. Once the community decide
that you have submitted enough patches (good patches), then we gonna
vote and promoted as a developer. We currently have people that submit
patches, and eventually we gonna promote them as developer :-)

Re: [enh] Configurable connect timeout in TCPConnectorHandler.

Hello guys,

> Piotr Bazan wrote:
>> Hey guys,
>>
>> I was thinking about a small enhancement in the code of the
>> TCPConnectorHandler. It would be useful from a user perspective to
>> have a connect timeout configurable and now it's fixed to 30 seconds.
>> Only a smooth change is required to enable this feature.
>
> Right now the value is defined by invoking
> DefaultSelectionKeyHandler.setTimeout(). Are you proposing to
> configure that value from the TCPConnectorHandler? I agree the current
> implementation is not intuitive :-)

I think Piotr is talking about following:

/ try {
isConnectedLatch.await(30, TimeUnit.SECONDS);
} catch (InterruptedException ex) {
throw new IOException(ex.getMessage());
}
/
I agree this looks strange. As we actually have onConnect method of
callback handler, which will be called once async. connect will be
completed. From other side we have this "always blocking" connect.
Looks here we have bad design :(
I can propose to make some changes there. Add 2 methods to to
ConnectorHandler interface:
isBlocking()
setBlocking(boolean)

which will work similar way as SocketChannel.configureBlocking()

With that change, we will be able to have both: blocking/non-blocking
behavior on connection/read/write operations.
Then... introduce methods:
(1)
public long read(ByteBuffer byteBuffer) throws IOException;
public long write(ByteBuffer byteBuffer) throws IOException;

we can remain (2) as deprecated. So blocking/non-blocking behavior of
(1) will depend on current ConnectorHandler blocking mode.

The same with ConnectorHandler.connect() method it will be able to work
both in blocking/non-blocking modes.

What do you think?

Thanks.

WBR,
Alexey.

>
>
> What do you think about
>> it? Btw what are the steps for joining active development of the
>> framework?
>
> Just submit patches (svn diff) to dev@grizzly. Once the community
> decide that you have submitted enough patches (good patches), then we
> gonna vote and promoted as a developer. We currently have people that
> submit patches, and eventually we gonna promote them as developer :-)
>
> Thanks
>
> -- Jeanfrancois
>
>
>>
>> thanks,
>> Piotrek
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]>> For additional commands, e-mail: [hidden email]>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]> For additional commands, e-mail: [hidden email]>

Re: [enh] Configurable connect timeout in TCPConnectorHandler.

Salut,

Oleksiy Stashok wrote:

> Hello guys,
>
>> Piotr Bazan wrote:
>>> Hey guys,
>>>
>>> I was thinking about a small enhancement in the code of the
>>> TCPConnectorHandler. It would be useful from a user perspective to
>>> have a connect timeout configurable and now it's fixed to 30 seconds.
>>> Only a smooth change is required to enable this feature.
>>
>> Right now the value is defined by invoking
>> DefaultSelectionKeyHandler.setTimeout(). Are you proposing to
>> configure that value from the TCPConnectorHandler? I agree the current
>> implementation is not intuitive :-)
> I think Piotr is talking about following:
>
> / try {
> isConnectedLatch.await(30, TimeUnit.SECONDS);
> } catch (InterruptedException ex) {
> throw new IOException(ex.getMessage());
> }
> /
> I agree this looks strange. As we actually have onConnect method of
> callback handler, which will be called once async. connect will be
> completed. From other side we have this "always blocking" connect.
> Looks here we have bad design :(

Well, the semantic of connect() right now is it will block until the
finishConnect() is invoked. The documentation should have stated that
the operation will blocks (this is the bad design ;-)). I would still
support a blocking connect (and a non blocking) based on your proposal
below.

> I can propose to make some changes there. Add 2 methods to to
> ConnectorHandler interface:
> isBlocking()
> setBlocking(boolean)
>
> which will work similar way as SocketChannel.configureBlocking()
>
> With that change, we will be able to have both: blocking/non-blocking
> behavior on connection/read/write operations.
> Then... introduce methods:
> (1)
> public long read(ByteBuffer byteBuffer) throws IOException;
> public long write(ByteBuffer byteBuffer) throws IOException;
>
> instead of:
> (2)
> public long read(ByteBuffer byteBuffer, boolean blocking) throws
> IOException;
> public long write(ByteBuffer byteBuffer, boolean blocking) throws
> IOException;
> we can remain (2) as deprecated. So blocking/non-blocking behavior of
> (1) will depend on current ConnectorHandler blocking mode.
>
> The same with ConnectorHandler.connect() method it will be able to work
> both in blocking/non-blocking modes.
>
> What do you think?

Looks good. Can you send a diff of the changes you propose? I agree it
will be quite similar to configureBlocking() method.

Re: [enh] Configurable connect timeout in TCPConnectorHandler.

+1 on JF comments.

And, it's also starting to look more and more like one of more
significant "bugs" is, documentation in general.

charlie ...

Jeanfrancois Arcand wrote:

> Salut,
>
> Oleksiy Stashok wrote:
>> Hello guys,
>>
>>> Piotr Bazan wrote:
>>>> Hey guys,
>>>>
>>>> I was thinking about a small enhancement in the code of the
>>>> TCPConnectorHandler. It would be useful from a user perspective to
>>>> have a connect timeout configurable and now it's fixed to 30
>>>> seconds. Only a smooth change is required to enable this feature.
>>>
>>> Right now the value is defined by invoking
>>> DefaultSelectionKeyHandler.setTimeout(). Are you proposing to
>>> configure that value from the TCPConnectorHandler? I agree the
>>> current implementation is not intuitive :-)
>> I think Piotr is talking about following:
>>
>> / try {
>> isConnectedLatch.await(30, TimeUnit.SECONDS);
>> } catch (InterruptedException ex) {
>> throw new IOException(ex.getMessage());
>> }
>> /
>> I agree this looks strange. As we actually have onConnect method of
>> callback handler, which will be called once async. connect will be
>> completed. From other side we have this "always blocking" connect.
>> Looks here we have bad design :(
>
> Well, the semantic of connect() right now is it will block until the
> finishConnect() is invoked. The documentation should have stated that
> the operation will blocks (this is the bad design ;-)). I would still
> support a blocking connect (and a non blocking) based on your proposal
> below.
>
>> I can propose to make some changes there. Add 2 methods to to
>> ConnectorHandler interface:
>> isBlocking()
>> setBlocking(boolean)
>>
>> which will work similar way as SocketChannel.configureBlocking()
>>
>> With that change, we will be able to have both: blocking/non-blocking
>> behavior on connection/read/write operations.
>> Then... introduce methods:
>> (1)
>> public long read(ByteBuffer byteBuffer) throws IOException;
>> public long write(ByteBuffer byteBuffer) throws IOException;
>>
>> instead of:
>> (2)
>> public long read(ByteBuffer byteBuffer, boolean blocking) throws
>> IOException;
>> public long write(ByteBuffer byteBuffer, boolean blocking) throws
>> IOException;
>> we can remain (2) as deprecated. So blocking/non-blocking behavior of
>> (1) will depend on current ConnectorHandler blocking mode.
>>
>> The same with ConnectorHandler.connect() method it will be able to
>> work both in blocking/non-blocking modes.
>>
>> What do you think?
>
> Looks good. Can you send a diff of the changes you propose? I agree it
> will be quite similar to configureBlocking() method.
>
> Thanks!
>
> -- Jeanfrancois
>
>
>>
>> Thanks.
>>
>> WBR,
>> Alexey.
>>
>>>
>>>
>>> What do you think about
>>>> it? Btw what are the steps for joining active development of the
>>>> framework?
>>>
>>> Just submit patches (svn diff) to dev@grizzly. Once the community
>>> decide that you have submitted enough patches (good patches), then
>>> we gonna vote and promoted as a developer. We currently have people
>>> that submit patches, and eventually we gonna promote them as
>>> developer :-)
>>>
>>> Thanks
>>>
>>> -- Jeanfrancois
>>>
>>>
>>>>
>>>> thanks,
>>>> Piotrek
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: [hidden email]>>>> For additional commands, e-mail: [hidden email]>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email]>>> For additional commands, e-mail: [hidden email]>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]>> For additional commands, e-mail: [hidden email]>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]> For additional commands, e-mail: [hidden email]>