Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7065 )

Change subject: IMPALA-5384: Simplify coordinator locking protocol
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7065/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/7065/2/be/src/runtime/coordinator.cc@487
PS2, Line 487:     ReleaseResources();
Started looking at resurrecting the ExecState part of this patch (the rest has 
been committed in various commits).  From reading through he code, I think 
there is a race between these two state transitions:

EXECUTING -> RETURNED_RESULTS
RETURNED_RESULTS -> ERROR_AFTER_RETURNED_RESULTS

The tail end of those transitions (after lock is dropped) can race so that 
there are concurrent executions of ReleaseResources(), ComputeQuerySummary(), 
etc.

We should be able to eliminate this race by eliminating the 
ERROR_AFTER_RETURNED_RESULTS state by having RETURNED_RESULTS also cause 
backend cancellation -- however, that will require untangling the whole 
ComputeQuerySummary mess to make sure we get full profiles for successful 
queries.

Alternatively, as a transitional step we might be able to remove that state 
without cancelling during RETURNED_RESULTS -- I think that's effectively how 
the code works today given the !returned_all_results_ check in 
UpdateBackendExecStatus() (that is arguable a bug, though).

In other words, make it so terminal states are never transitioned out of to 
another terminal state (which guarantees the terminal state handling code is 
executing exactly once).



--
To view, visit http://gerrit.cloudera.org:8080/7065
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a79aa38e529d0994921906b2beb796fd589a7e3
Gerrit-Change-Number: 7065
Gerrit-PatchSet: 2
Gerrit-Owner: Marcel Kornacker <marc...@gmail.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marc...@gmail.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Apr 2018 18:05:45 +0000
Gerrit-HasComments: Yes

Reply via email to