[
https://issues.apache.org/jira/browse/IGNITE-7285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16916143#comment-16916143
]
Ivan Pavlukhin commented on IGNITE-7285:
----------------------------------------
[~samaitra],
bq. 1. I ran tests for Distributed Query and local query like
IgniteCacheReplicatedQuerySelfTest,
IgniteCacheLocalQueryCancelOrTimeoutSelfTest and I see that timeout value is
passed. Can you share an example in which Query or execution mode you can see
that timeout is not passed?
Good to know that it works for different cases. I am concerned about timeout
propagation
{{org.apache.ignite.internal.processors.query.h2.CommandProcessor#processTxCommand}}.
It seems that _default timeout_ is not used in that class in PR. But I must
say that I do not fully understand why query timeout is used by transactional
commands.
bq. 2. I observed that QueryParameters initialization is occurring in
QueryParameters.fromQuery(SqlFieldsQuery qry) method and QueryParameters does
not have GridKernalContext reference so it will not be feasible to access the
ctx.config().getDefaultQueryTimeout() upon QueryParameters initialization.
{{org.apache.ignite.internal.processors.query.h2.QueryParameters#fromQuery}} is
used only in {{QueryParser}}, we can make it aware of ignite configuration by
placing {{fromQuery}} method to {{QueryParser}} or by introducing a class
responsible for {{QueryParameters}} construction in presence of default values
in configuration (only timeout so far). Are there major disadvantages of such
approach?
.bq If you observe timeout in SqlFieldsQuery get validated in
QueryUtils.validateTimeout(timeout, timeUnit) and it asserts that timeout value
of SqlFieldsQuery is more than equal 0. Now if we allow feature to disable
timeout of particular query using (SqlFieldsQuery.setTimeout(0)) then
ctx.config().getDefaultQueryTimeout() will never get used.
I understood that disabling a timeout in presence of non-zero default value is
not so trivial. But I am still concerned how usable it would be? It is
desirable to protect a user from surprising behavior of {{setTimeout(0)}} when
default timeout is configured. Actually, to distinguish between _unset_ vs
_disabled_ timeout I thought about using -1 to indicate that a user did not
call {{setTimeout}}. Of course some care is needed to implement such approach
properly.
One more additional note. We have
{{org.apache.ignite.internal.sql.command.SqlBulkLoadCommand}} and we should
consider a _default timeout_ for it, at least roughly estimate how challenging
is it.
> Add default query timeout
> -------------------------
>
> Key: IGNITE-7285
> URL: https://issues.apache.org/jira/browse/IGNITE-7285
> Project: Ignite
> Issue Type: Improvement
> Components: cache, sql
> Affects Versions: 2.3
> Reporter: Valentin Kulichenko
> Assignee: Saikat Maitra
> Priority: Major
> Labels: sql-stability
> Fix For: 2.8
>
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> Currently it's possible to provide timeout only on query level. It would be
> very useful to have default timeout value provided on cache startup. Let's
> add {{CacheConfiguration#defaultQueryTimeout}} configuration property.
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
