Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20986 )
Change subject: IMPALA-12782: Show info of the event processing in /events webUI ...................................................................... Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/20986/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20986/8//COMMIT_MSG@12 PS8, Line 12: processing nit: processed http://gerrit.cloudera.org:8080/#/c/20986/8/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/20986/8/be/src/catalog/catalog-server.cc@980 PS8, Line 980: last_synced_event_id idea to reduce the chattiness of the code: a struct like ObjWrapper could be created with a template function AddMember(const char* name, const T& val), specialized for std::string. This would allow skipping creating local Value variables and passing the allocator in each AddMember(). So this could look like: ObjWrapper progress_info_obj(allocator); progress_info_obj.AddMember("last_synced_event_id", progress_info.last_synced_event_id) ... document->AddMember("progress-info", progress_info_obj.value, allocator); Note that RapidJson uses move semanctics, so after Value::AddMember("s", val, allocator), 'val' is set to null, so there is no need to keep it on the stack. http://gerrit.cloudera.org:8080/#/c/20986/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java: http://gerrit.cloudera.org:8080/#/c/20986/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1192 PS3, Line 1192: * @throws MetastoreNotificationException > I remove calling it in finally since it will reset currentEvent_ for except One member when this seems problematic is currentEventIndex_ - if there is an exception then it won't be reset, so after restarting the event processor currentEventIndex_ won't start from zero in the first batch. What do you think about calling resetProgress() in finally, but in case of an exception, saving the the current event to a member like lastEventDuringException_? This could be returned instead of currentFilteredEvent_ in case currentFilteredEvent_ and the status is error. It could be also useful do display the last problematic event in webui, even if the event processor was restarted. http://gerrit.cloudera.org:8080/#/c/20986/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java: http://gerrit.cloudera.org:8080/#/c/20986/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1146 PS8, Line 1146: in case they are replaced : // when being used below "in case they are replaced concurrently"? It could be also mentioned that the consistency of the members in progressInfo is not guaranteed in case of parallel modifications. http://gerrit.cloudera.org:8080/#/c/20986/8/tests/custom_cluster/test_web_pages.py File tests/custom_cluster/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/20986/8/tests/custom_cluster/test_web_pages.py@444 PS8, Line 444: catalogd_event_processing_delay Probably out of the scope of this patch, but a potential alternative would be to use table properties to inject debug actions to the event processor, so that the debug action would be triggered only for events from the specific table. This would allow these tests to run without using a custom cluster. -- To view, visit http://gerrit.cloudera.org:8080/20986 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e7d4952c7fd04ae89b6751204499bf9dd99f57c Gerrit-Change-Number: 20986 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Wed, 13 Mar 2024 16:02:54 +0000 Gerrit-HasComments: Yes
