Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22250 )

Change subject: IMPALA-12785: Add commands to control event-processor status
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/22250/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22250/8//COMMIT_MSG@10
PS8, Line 10: EventProcessor. E.g. to pause the EventProcessor:
            :   impala-shell> :event_processor('pause');
            : to resume the EventProcessor:
            :   impala-shell> :event_processor('start');
Why not implement this as two separate command: event_processor_pause and 
event_processor_start?


http://gerrit.cloudera.org:8080/#/c/22250/8//COMMIT_MSG@20
PS8, Line 20: pause, start, status
Do we have server-side validation for this possible strings?
TEventProcessorCmdParams.action is string.


http://gerrit.cloudera.org:8080/#/c/22250/8//COMMIT_MSG@27
PS8, Line 27: LastSyncedEventId: 34489. LatestEventId: 34489.
Can you document in commit message what is the difference between 
LastSyncedEventId and LatestEventId? Does :event_processor('status'); display 
them?


http://gerrit.cloudera.org:8080/#/c/22250/8//COMMIT_MSG@34
PS8, Line 34: Note that resuming EventProcessor back to a previous event id is 
only
            : allowed when it's not in the ACTIVE state.
Why not make it possible? What is the potential harm?
It just mean that server need to do pause followed by start, and that is more 
convenient & fast for admin than running this command twice. Maybe not atomic, 
but convenient.

Can :event_processor('status'); fail?


http://gerrit.cloudera.org:8080/#/c/22250/8/common/thrift/CatalogService.thrift
File common/thrift/CatalogService.thrift:

http://gerrit.cloudera.org:8080/#/c/22250/8/common/thrift/CatalogService.thrift@758
PS8, Line 758:  required string action
Document what is the possible actions are.
Why this is not an enum?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a19f67264cfe06a1819a22c0c4f0cf174c9b958
Gerrit-Change-Number: 22250
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[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: Riza Suminto <[email protected]>
Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]>
Gerrit-Comment-Date: Wed, 22 Jan 2025 19:22:46 +0000
Gerrit-HasComments: Yes

Reply via email to