Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8758 )
Change subject: IMPALA-6190/6246: Add instances tab and event sequence ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/8758/2/be/src/runtime/fragment-instance-state.cc File be/src/runtime/fragment-instance-state.cc: http://gerrit.cloudera.org:8080/#/c/8758/2/be/src/runtime/fragment-instance-state.cc@296 PS2, Line 296: bool first_batch_received = false; : bool first_batch_sent = false; : do { : Status status; : row_batch_->Reset(); : { : if (!first_batch_received) { : UpdateCurrentState(TFInstanceState::WAITING_FOR_FIRST_BATCH); : } : SCOPED_TIMER(plan_exec_timer); : RETURN_IF_ERROR( : exec_tree_->GetNext(runtime_state_, row_batch_.get(), &exec_tree_complete)); : if (!first_batch_received) { : first_batch_received = true; : UpdateCurrentState(TFInstanceState::FIRST_BATCH_RECEIVED); : } else if (current_state_.load() != TFInstanceState::RECEIVING_DATA) { : UpdateCurrentState(TFInstanceState::RECEIVING_DATA); : } : } : if (VLOG_ROW_IS_ON) row_batch_->VLogRows("FragmentInstanceState::ExecInternal()"); : COUNTER_ADD(rows_produced_counter_, row_batch_->num_rows()); : RETURN_IF_ERROR(sink_->Send(runtime_state_, row_batch_.get())); : if (!first_batch_sent) { : first_batch_sent = true; : UpdateCurrentState(TFInstanceState::FIRST_BATCH_SENT); : } : } while (!exec_tree_complete); : : UpdateCurrentState(TFInstanceState::LAST_BATCH_SENT); > I think all the state transition logic would be easier to follow if we expl To be clear, the goal is to remove all the if-stmts and state variables from this code. The transitions would be something like GET_NEXT SEND EOS or whatever, and so UpdateCurrentState(GET_NEXT) and UpdateCurrentState(SEND), etc would get called repeatedly, but the state machine logic would take care of knowing how to deal with that transition given the current state. -- To view, visit http://gerrit.cloudera.org:8080/8758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I626456b6afa9101eeeeffd5cda10c4096d63d7f9 Gerrit-Change-Number: 8758 Gerrit-PatchSet: 2 Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Comment-Date: Tue, 05 Dec 2017 19:39:25 +0000 Gerrit-HasComments: Yes
