Skip to content

Conversation

@pvary
Copy link
Contributor

@pvary pvary commented Nov 12, 2020

Running the iceberg-mr/iceberg-hive3 tests are beginning to take serious amount of time.
This is partially because every tests are run 24 times (2 Engines X 3 FileFormats X 4 Catalogs). We can separate out some of the tests which does not depend the FileFormat/Catalog and this way we can save time.

Also the Catalog tests are still using inheritance which we can replace with parametrized tests for cleaner code.

The current patch is above #1612 since I do not want to rebase that patch again 😄

@rdblue
Copy link
Contributor

rdblue commented Nov 25, 2020

@pvary, what's the status here? I took a quick look at the changes and I see some timestamp changes mixed in?

@pvary
Copy link
Contributor Author

pvary commented Nov 25, 2020

@pvary, what's the status here? I took a quick look at the changes and I see some timestamp changes mixed in?

This patch is changing the big test class used by most of the Hive/Tez/Mr tests (HiveIcebergStorageHandlerBaseTest), so this messes up with most of the Hive patches. I based this patch on the current version of CREATE TABLE PR (#1612) at that time, because I did not want to mess-up with the review there. Of course this patch is outdated since then, and I will have to rebase once #1612 gets in.

If the the test is independed of the FileFormat/Execution engine then only run ti on every Catalog
@pvary
Copy link
Contributor Author

pvary commented Nov 27, 2020

Created #1840 (Hive: Refactor HiveIcebergStorageHandler tests to use catalogs as parameters) as a first part of this change

@rdblue
Copy link
Contributor

rdblue commented Jan 5, 2021

I ran into this issue over the break and found that on my local machine, the storage handler tests were taking 11 minutes to run and running for both Hive 2 and Hive 3. It would be great to get this refactor done and not run so many expensive cases!

@pvary pvary closed this Jan 7, 2021
@pvary pvary deleted the fixtest branch January 7, 2021 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants