Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: concurrent access to RestSqlQueryAction is not thread safe #69866

Closed
astefan opened this issue Mar 3, 2021 · 2 comments
Closed

SQL: concurrent access to RestSqlQueryAction is not thread safe #69866

astefan opened this issue Mar 3, 2021 · 2 comments
Assignees
Labels
:Analytics/SQL SQL querying >bug Team:QL (Deprecated) Meta label for query languages team

Comments

@astefan
Copy link
Contributor

astefan commented Mar 3, 2021

RestSqlQueryAction has a TextFormat variable defined at instance level. This one can be overriden by two threads running in parallel servicing REST requests with prepareRequest calls where the textFormat value is changed.

This means that one call looking like /_sql?format=txt (textFormat here gets the value TXT) and another like /_sql (textFormat here remains null since xContentType gets a value of JSON) running at the same time makes the textFormat for the first call to be set to null and an exception like the following is getting logged and the call fails:

[WARN ][r.suppressed             ] [runTask-0] path: /_sql, params: {format=txt}
java.lang.NullPointerException: Cannot invoke "org.elasticsearch.xpack.sql.plugin.TextFormat.format(org.elasticsearch.rest.RestRequest, org.elasticsearch.xpack.sql.action.SqlQueryResponse)" because "this.this$0.textFormat" is null
        at org.elasticsearch.xpack.sql.plugin.RestSqlQueryAction$1.buildResponse(RestSqlQueryAction.java:130) ~[?:?]
        at org.elasticsearch.xpack.sql.plugin.RestSqlQueryAction$1.buildResponse(RestSqlQueryAction.java:117) ~[?:?]
        at org.elasticsearch.rest.action.RestResponseListener.processResponse(RestResponseListener.java:26) ~[elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.rest.action.RestActionListener.onResponse(RestActionListener.java:36) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.support.TransportAction$1.onResponse(TransportAction.java:83) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.support.TransportAction$1.onResponse(TransportAction.java:77) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.xpack.sql.plugin.TransportSqlQueryAction.lambda$operation$0(TransportSqlQueryAction.java:97) [x-pack-sql-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT
        at org.elasticsearch.action.ActionListener$1.onResponse(ActionListener.java:134) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.ActionListener$1.onResponse(ActionListener.java:134) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.ActionListener$1.onResponse(ActionListener.java:134) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.xpack.sql.execution.search.CompositeAggCursor.handle(CompositeAggCursor.java:196) [x-pack-sql-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.xpack.sql.execution.search.Querier$CompositeActionListener.handleResponse(Querier.java:389) [x-pack-sql-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.xpack.sql.execution.search.Querier$BaseActionListener.onResponse(Querier.java:558) [x-pack-sql-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.xpack.sql.execution.search.Querier$BaseActionListener.onResponse(Querier.java:532) [x-pack-sql-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.support.TransportAction$1.onResponse(TransportAction.java:83) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.support.TransportAction$1.onResponse(TransportAction.java:77) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.ActionListener$RunAfterActionListener.onResponse(ActionListener.java:313) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.AbstractSearchAsyncAction.sendSearchResponse(AbstractSearchAsyncAction.java:630) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.ExpandSearchPhase.run(ExpandSearchPhase.java:109) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.AbstractSearchAsyncAction.executePhase(AbstractSearchAsyncAction.java:397) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.AbstractSearchAsyncAction.executeNextPhase(AbstractSearchAsyncAction.java:391) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.FetchSearchPhase.moveToNextPhase(FetchSearchPhase.java:219) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.FetchSearchPhase.lambda$innerRun$1(FetchSearchPhase.java:101) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.FetchSearchPhase.innerRun(FetchSearchPhase.java:107) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.FetchSearchPhase.access$000(FetchSearchPhase.java:36) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.action.search.FetchSearchPhase$1.doRun(FetchSearchPhase.java:84) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:26) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.common.util.concurrent.TimedRunnable.doRun(TimedRunnable.java:33) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:732) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:26) [elasticsearch-7.13.0-SNAPSHOT.jar:7.13.0-SNAPSHOT]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130) [?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630) [?:?]
        at java.lang.Thread.run(Thread.java:832) [?:?]

Also, at the time this bug report was created, the code in master and the one in 7.x differ significantly when it comes to the parts relevant for this bug, so a fix should address 7.x code only.

@astefan astefan added >bug :Analytics/SQL SQL querying labels Mar 3, 2021
@astefan astefan self-assigned this Mar 3, 2021
@elasticmachine elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Mar 3, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

@astefan
Copy link
Contributor Author

astefan commented Mar 3, 2021

This is fixed with #69895.

@astefan astefan closed this as completed Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/SQL SQL querying >bug Team:QL (Deprecated) Meta label for query languages team
Projects
None yet
Development

No branches or pull requests

2 participants