[DISCUSS] serializedResponseTimeout usage

[Stephen Mallette <sp...@gmail.com>]

Not sure if anyone will have an opinions on this, but as Gremlin Server has
hardened its capabilities over the last several releases, I've come to
notice that the serializedResponseTimeout setting isn't demonstrating much
use. When this setting was introduced there were a number of individual
timeouts for different parts of request/response processing and as time has
progressed we've sorta narrowed down to only needing the
scriptEvaluationTimeout.

As it stands right now, the serializedResponseTimeout is a timeout that can
occur within the scriptEvaluationTimeout. In other words, the
serializedResponseTimeout is essentially ignored if the it is larger than
the scriptEvaluationTimeout and can lead to misleading errors if it is
smaller. On top of that, I really can't think of a good use case for when
you would want it to be smaller in the first place. To me the
scriptEvaluationTimeout can handle everything just fine which would give
users just one timeout to worry about.

I think I'd like to remove serializedResponseTimeout from the available
settings. In a sense this would be a breaking change in that the feature
would no longer be present. I think I could deprecate the property in the
Gremlin Server Settings class so that anyone programmatically starting
Gremlin Server would not have a breaking change to deal with. That
deprecation could have some javadoc that simply said that this feature is
no longer in use and to simply set the scriptEvaluationTimeout. I would
presume that most people are setting the scriptEvaluationTimeout at this
point anyway as that is the most discussed setting.

If there are any concerns about going this direction, then please raise
them. If not, I'll assume lazy consensus in the next 72 hours (Saturday
April 30, 2016 9:15 am) and move forward with the change.

Re: [DISCUSS] serializedResponseTimeout usage

[Stephen Mallette <sp...@gmail.com>]

Sorry - while i was writing the JIRA ticket and linking to this DISCUSS
thread in it, i realized that we could keep this from being a breaking
change at all and just allow for pure deprecation if we provide a way to
just disable timeout and then add the deprecation annotations/javadoc. We
would use the same method for disabling the serializedResponseTimeout as we
already have for the scriptEvaluationTimeout - setting it to a negative
number. Then we can just make the default setting be "disabled".

On Wed, Apr 27, 2016 at 9:21 AM, Stephen Mallette <sp...@gmail.com>
wrote:

> Not sure if anyone will have an opinions on this, but as Gremlin Server
> has hardened its capabilities over the last several releases, I've come to
> notice that the serializedResponseTimeout setting isn't demonstrating much
> use. When this setting was introduced there were a number of individual
> timeouts for different parts of request/response processing and as time has
> progressed we've sorta narrowed down to only needing the
> scriptEvaluationTimeout.
>
> As it stands right now, the serializedResponseTimeout is a timeout that
> can occur within the scriptEvaluationTimeout. In other words, the
> serializedResponseTimeout is essentially ignored if the it is larger than
> the scriptEvaluationTimeout and can lead to misleading errors if it is
> smaller. On top of that, I really can't think of a good use case for when
> you would want it to be smaller in the first place. To me the
> scriptEvaluationTimeout can handle everything just fine which would give
> users just one timeout to worry about.
>
> I think I'd like to remove serializedResponseTimeout from the available
> settings. In a sense this would be a breaking change in that the feature
> would no longer be present. I think I could deprecate the property in the
> Gremlin Server Settings class so that anyone programmatically starting
> Gremlin Server would not have a breaking change to deal with. That
> deprecation could have some javadoc that simply said that this feature is
> no longer in use and to simply set the scriptEvaluationTimeout. I would
> presume that most people are setting the scriptEvaluationTimeout at this
> point anyway as that is the most discussed setting.
>
> If there are any concerns about going this direction, then please raise
> them. If not, I'll assume lazy consensus in the next 72 hours (Saturday
> April 30, 2016 9:15 am) and move forward with the change.
>