[
https://issues.apache.org/jira/browse/DRILL-6435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16488292#comment-16488292
]
ASF GitHub Bot commented on DRILL-6435:
---------------------------------------
vrozov commented on issue #1286: DRILL-6435: MappingSet is stateful, so it
can't be shared between threads
URL: https://github.com/apache/drill/pull/1286#issuecomment-391560716
> The specific fix may be OK, but I believe it overlooks a broader point.
The `MappingSet` is supposed to be a static association of elements used in
code generation. This mapping is meant to be static and thus sharable.
> If so, then making the objects non-static makes sense; but I wonder what
other things will be broken since the original design appeared to be that
`MappingSets` are static.
I don't see `MappingSet` being designed to be `static` and thus sharable
between threads. The state of the object is modified during code generation
(expression compilation) and if there are multiple queries submitted for code
generation at the same time, the state machine of the `MappingSet` goes wild as
a calls to `enterConstant()` may be followed by a call to `enterConstant()` for
a different query on a different thread (I am not even worried about
multi-thread safety at this point).
> If there is an issue, then somewhere we must have added code that alters
these objects per-query. We might want to back up and find that use, then ask
if it is really necessary.
The code that modifies the state of the `MappingSet` object during code
generation was always there. It is not a newly introduced code.
> At the very least, perhaps we'd want to add a comment explaining why we
need per-query modifications rather than the standard, static definitions.
`static` definition for `MappingSet` is not standard. In the majority of
places it was already declared as instance variable (not `static`).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> MappingSet is stateful, so it can't be shared between threads
> -------------------------------------------------------------
>
> Key: DRILL-6435
> URL: https://issues.apache.org/jira/browse/DRILL-6435
> Project: Apache Drill
> Issue Type: Bug
> Reporter: Vlad Rozov
> Assignee: Vlad Rozov
> Priority: Major
>
> There are several instances where static {{MappingSet}} instances are used
> (for example {{NestedLoopJoinBatch}} and {{BaseSortWrapper}}). This causes
> instance reuse across threads when queries are executed concurrently. As
> {{MappingSet}} is a stateful class with visitor design pattern, such reuse
> causes invalid state.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
