Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/22459 )
Change subject: IMPALA-13611: Add interop tests for Iceberg tables ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/22459/1/tests/stress/test_update_stress.py File tests/stress/test_update_stress.py: http://gerrit.cloudera.org:8080/#/c/22459/1/tests/stress/test_update_stress.py@215 PS1, Line 215: """Increments j's value with each iteration until all rows are deleted""" > This comment doesn't seem to be for this function. Neither 'num_inserts' no Done http://gerrit.cloudera.org:8080/#/c/22459/1/tests/stress/test_update_stress.py@226 PS1, Line 226: ve_client.close() > Did you check if the number if deletes and updates are balanced also within It's hard to tell what is balanced in this situation. My observation is that if one of the writer's clocks is not shifted a bit, they lock each other from committing. If I keep Hive's interval 7 times higher then Impala's deleted job will do multiple deletes, and Hive can do one update at least. I know it's not optimal, but this case will run for a lot more time without shifting. http://gerrit.cloudera.org:8080/#/c/22459/1/tests/stress/test_update_stress.py@282 PS1, Line 282: t( > num_rows Done http://gerrit.cloudera.org:8080/#/c/22459/1/tests/stress/test_update_stress.py@358 PS1, Line 358: """Issues DELETE statements from Impala and UPDATE statements from Hive > Couldn't the implementations of these two functions be merged, with the del The whole class could be refactored to make the operators interchangeable, I followed the pattern that is already used here. I think, for tests, simplicity and readability are more important than the code size. I can rewrite it, but in this case, I'd refactor the whole class, and I could not promise a better result at the end. http://gerrit.cloudera.org:8080/#/c/22459/1/tests/stress/test_update_stress.py@409 PS1, Line 409: > Is it intentional that test_iceberg_impala_deletes_and_hive_updates doesn't Yes, in this case, Hive's deletion must be processed by event processing or by an explicit REFRESH. In this case, I choose REFRESH. The other case will always show the correct result without REFRESH as the last "updater" of the table is Impala. -- To view, visit http://gerrit.cloudera.org:8080/22459 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2ee6d3354c3b11264c5e3ded9826831e3962a98 Gerrit-Change-Number: 22459 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Rozsa <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Mon, 24 Feb 2025 17:57:47 +0000 Gerrit-HasComments: Yes
