[Impala-ASF-CR] IMPALA-5384: Simplify coordinator locking protocol

["Dan Hecht (Code Review)" <ge...@cloudera.org>]

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 <ma...@gmail.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@gmail.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Apr 2018 18:05:45 +0000
Gerrit-HasComments: Yes