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

Change subject: IMPALA-14082: Support batch processing of RELOAD events on same 
table
......................................................................


Patch Set 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/23159/6/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/23159/6/tests/custom_cluster/test_events_custom_configs.py@578
PS6, Line 578:
> reload events don't rely on selfevent() (instead they rely on isOlderEvent())

It's not a change of this patch. Could you explain how the test pass before 
this patch and add a comment please?


http://gerrit.cloudera.org:8080/#/c/23159/6/tests/custom_cluster/test_events_custom_configs.py@647
PS6, Line 647:       req.partitionVals = ["2022"]
> Previously reload partition is processed one by one, so when batch events a
So the purpose of this test is not matched anymore since no events are skipped. 
Since we now have commands to pause event processing, maybe we can test it this 
way:

1. pause event processing
2. fire RELOAD event
3. run REFRESH
4. resume event processing and verify the RELOAD event is skipped.


http://gerrit.cloudera.org:8080/#/c/23159/7/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/23159/7/tests/custom_cluster/test_events_custom_configs.py@631
PS7, Line 631:       # Test to verify if older events are being skipped in 
event processor
Let's add a comment to explain how the test works. IIUC, we fire 10 consecutive 
RELOAD events. Processing the first one updates the lastRefreshEventId which 
helps to skip the other 9 events.

But why does it only work when enable_sync_to_latest_event_on_ddls=true?


http://gerrit.cloudera.org:8080/#/c/23159/7/tests/custom_cluster/test_events_custom_configs.py@808
PS7, Line 808:     self.client.execute("create table {} (i int) partitioned 
by(p int)".format(tbl))
Let's create the table as text format so we can read the new files correctly.


http://gerrit.cloudera.org:8080/#/c/23159/7/tests/custom_cluster/test_events_custom_configs.py@818
PS7, Line 818: 000
Let's use a different value other than 0 so we can verify the content as well. 
Also delete the file under p=1 to verify file deletion. And it'd be nice if we 
can overwrite the file under p=2.

At the end of this test we can verify the results of SELECT * FROM this table.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3e9a99b666a1c928ac2a136bded1e5420f77dab
Gerrit-Change-Number: 23159
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <[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: Sat, 06 Sep 2025 12:54:50 +0000
Gerrit-HasComments: Yes

Reply via email to