Skip to content

Add test for metastore access count in Delta Lake#11973

Merged
findepi merged 2 commits intotrinodb:masterfrom
ebyhr:ebi/delta-metastore-access
Apr 21, 2022
Merged

Add test for metastore access count in Delta Lake#11973
findepi merged 2 commits intotrinodb:masterfrom
ebyhr:ebi/delta-metastore-access

Conversation

@ebyhr
Copy link
Copy Markdown
Member

@ebyhr ebyhr commented Apr 15, 2022

Description

Add test for metastore access count in Delta Lake

Documentation

(x) No documentation is needed.

Release notes

(x) No release notes entries required.

@ebyhr ebyhr added the no-release-notes This pull request does not require release notes entry label Apr 15, 2022
@cla-bot cla-bot bot added the cla-signed label Apr 15, 2022
@ebyhr ebyhr requested review from findepi and losipiuk April 15, 2022 10:22
@findepi
Copy link
Copy Markdown
Member

findepi commented Apr 15, 2022

@ebyhr see CI for delta-lake failed

@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Apr 16, 2022

@findepi Fixed CI failure.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from Iceberg's CountingAccessFileHiveMetastore, right?

Let's move the class to trino-hive/src/test/java and share via a test-jar dependency.
When moving, drop "File" from the name (it's a left over)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, moved to Hive module.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add before Module module, (as you did in TestingDeltaLakePlugin class)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, i think the change here is not required.
You can bind your own metastore via Module module, you just need to set hive.metastore to some unsupported value (eg other).

would that work? i think this is what TestDeltaLakeMetastoreAccessOperations is doing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a followup, this test class can perhaps be simplified with the CountingAccess{File}HiveMetastore class.
i.e. we could remove reflection use from here.

for that, we would need to ensure every method in CountingAccess{File}HiveMetastore is counted, not only selected ones.

@findepi findepi merged commit 4d7a094 into trinodb:master Apr 21, 2022
@github-actions github-actions bot added this to the 378 milestone Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

2 participants