Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20192 )

Change subject: IMPALA-12267: DMLs/DDLs can hang as a result of catalogd restart
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20192/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/20192/5/be/src/service/impala-server.cc@2267
PS5, Line 2267:         while 
(ShouldContinueWaitingForNewCatalogServiceId(cur_service_id, timed_out,
I think we don't need this new method. We can add two breaks in the loop for 
timeout and max statestore updates. E.g.

        while (catalog_update_info_.catalog_service_id == cur_service_id) {
          if (timeout_set) {
            timed_out = !catalog_version_update_cv_.WaitUntil(ver_lock, 
wait_end_time);
            if (timed_out) {
              LOG(WARNING) << "Waiting for catalog update with a new "
                           << "catalog service ID timed out.";
              break;
            }
          } else {
            catalog_version_update_cv_.Wait(ver_lock);
          }
          if (max_statestore_updates_set
              && catalog_update_info_.catalog_version != old_catalog_version) {
            if (catalog_update_info_.catalog_service_id != cur_service_id) 
break;
            old_catalog_version = catalog_update_info_.catalog_version;
            ++num_statestore_updates;
            if (num_statestore_updates < max_statestore_updates) {
              LOG(INFO) << "Received " << num_statestore_updates << " non-empty"
                  << " catalog updates from the statestore while waiting for an 
update "
                  << "with a new catalog service ID but the catalog service ID 
has not "
                  << "changed. Going to give up waiting after " << 
max_statestore_updates
                  << " such updates in total.";
            } else {
              LOG(WARNING) << "Received " << num_statestore_updates << " 
non-empty"
                  << " catalog updates from the statestore while waiting for an 
update "
                  << "with a new catalog service ID but the catalog service ID 
has not "
                  << "changed. Giving up waiting.";
              break;
            }
          }
        }

Note that we should invoke Wait() before counting the updates since when 
entering this loop there are no updates yet.
Also in the logs, it'd be better to emphasize it's a non-empty catalog update.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib71bec8f67f80b0bdfe0a6cc46a16ef624163d8b
Gerrit-Change-Number: 20192
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Mon, 24 Jul 2023 06:32:46 +0000
Gerrit-HasComments: Yes

Reply via email to