Procedure to invalidate directory list cache #19821

agrawalreetika · 2023-06-08T06:39:45Z

Test plan - (Please fill in how you tested your changes)

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines. Don't forget to follow our attribution guidelines for any code copied from other projects.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== RELEASE NOTES ==


Hive Changes
* Introduce system procedure to invalidate directory list cache in Hive Catalog

nmahadevuni · 2023-06-09T05:23:16Z

@agrawalreetika Instead of single directory, we may want to clear the whole cache too. May be make the directoryPath optional, and if its not provided, then clear all cache. Also, please change the procedure name to something more intuitive.

If there are multiple hive catalogs, how can we invalidate the cache from one particular catalog?

system.invalidate_file_list_cache(Optional<String> directoryPath)

agrawalreetika · 2023-06-12T05:18:02Z

@agrawalreetika Instead of single directory, we may want to clear the whole cache too. May be make the directoryPath optional, and if its not provided, then clear all cache. Also, please change the procedure name to something more intuitive.

If there are multiple hive catalogs, how can we invalidate the cache from one particular catalog?

system.invalidate_file_list_cache(Optional<String> directoryPath)

Hi @nmahadevuni, Thanks for your comment.
I have added the directory path as optional to allow full directory list cache invalidation.
Just like any other procedure in Presto we will have to provide catalog name while making procedure call on specific catalog. (ref - https://prestodb.io/docs/current/connector/hive.html#procedures)

agrawalreetika · 2023-06-14T14:10:09Z

@tdcmeehan Could you please help me review this?

imjalpreet

@agrawalreetika thanks for the PR, overall LGTM. A few minor nits.

presto-hive/src/main/java/com/facebook/presto/hive/CacheInvalidationProcedure.java

imjalpreet

LGTM 👍🏼

presto-hive/src/main/java/com/facebook/presto/hive/HiveClientModule.java

tdcmeehan · 2023-06-15T14:40:26Z

presto-hive/src/main/java/com/facebook/presto/hive/DirectoryListCacheInvalidationProcedure.java

+        }
+    }
+
+    private void doInvalidateDirectoryListCache(ConnectorSession session, Optional<String> directoryPath)


Can we improve the enapsulation of CachingDirectoryLister by moving this method to be inside of it? We could also write a simple unit test.

@tdcmeehan Since we are going to use this for the Directory list invalidation procedure, which is why I thought of keeping it here. Do you think we should move this to CachingDirectoryLister?

I have added tests.

I am in favor of keeping this inside of CachingDirectoryLister for improved encapsulation.

presto-hive/src/main/java/com/facebook/presto/hive/CachingDirectoryLister.java

mrmaddx2 · 2023-07-13T15:09:34Z

presto-hive/src/test/java/com/facebook/presto/hive/TestDirectoryListCacheInvalidation.java

+        assertEquals(missCount, 1);
+
+        // Invalidate directory list cache
+        assertQuerySucceeds("CALL hive.system.invalidate_directory_list_cache()");


would be nice to also test parametrized version.

presto-docs/src/main/sphinx/connector/hive.rst

github-actions · 2023-12-01T12:56:52Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff 908327e...004ba4b.

Notify	File(s)
@steveburnett	presto-docs/src/main/sphinx/connector/hive.rst

steveburnett

LGTM! (docs)
Local build of docs, new additions look good.

steveburnett · 2024-01-29T14:43:25Z

Failing on one test (test / test (:presto-hive) (pull_request), and no option to re-run failed tests.

tdcmeehan · 2024-02-27T16:18:28Z

presto-hive/src/main/java/com/facebook/presto/hive/CachingDirectoryLister.java

 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static java.util.Objects.requireNonNull;

 public class CachingDirectoryLister
        implements DirectoryLister
 {
+    protected final DirectoryLister delegate;


It looks like this is still protected

agrawalreetika · 2024-02-28T12:12:09Z

@tdcmeehan I have been facing this issue in product-tests-basic-environment here but not finding the details around what's causing this.
Are there any more logs that get captured as part of workflow run?

tdcmeehan · 2024-02-28T14:17:31Z

@agrawalreetika why do we need new product tests for this feature? Can't it run in an existing suite?

agrawalreetika · 2024-02-28T19:54:11Z

@agrawalreetika why do we need new product tests for this feature? Can't it run in an existing suite?

@tdcmeehan I added tests to the existing suite, but looks like product-tests-basic-environment is having same issue here

agrawalreetika · 2024-03-06T16:13:29Z

@tdcmeehan & @steveburnett all the tests are green, this is ready for final review.

steveburnett

LGTM! (docs)

Pulled updated branch, new local build, docs looks good. Thanks!

agrawalreetika requested a review from a team as a code owner June 8, 2023 06:39

agrawalreetika requested a review from presto-oss June 8, 2023 06:39

agrawalreetika self-assigned this Jun 8, 2023

agrawalreetika force-pushed the list-cache-invalidation branch from f95b59e to cec513e Compare June 12, 2023 05:15

agrawalreetika force-pushed the list-cache-invalidation branch from cec513e to 06fb04f Compare June 14, 2023 13:29

imjalpreet requested changes Jun 15, 2023

View reviewed changes

presto-hive/src/main/java/com/facebook/presto/hive/CacheInvalidationProcedure.java Outdated Show resolved Hide resolved

presto-hive/src/main/java/com/facebook/presto/hive/CacheInvalidationProcedure.java Outdated Show resolved Hide resolved

agrawalreetika force-pushed the list-cache-invalidation branch from 06fb04f to 05cafc7 Compare June 15, 2023 12:42

imjalpreet previously approved these changes Jun 15, 2023

View reviewed changes

tdcmeehan requested changes Jun 15, 2023

View reviewed changes

agrawalreetika force-pushed the list-cache-invalidation branch from 05cafc7 to 9433002 Compare June 19, 2023 06:48

imjalpreet reviewed Jun 22, 2023

View reviewed changes

presto-hive/src/main/java/com/facebook/presto/hive/CachingDirectoryLister.java Outdated Show resolved Hide resolved

agrawalreetika force-pushed the list-cache-invalidation branch from 9433002 to 2bf4b22 Compare July 12, 2023 06:02

mrmaddx2 reviewed Jul 13, 2023

View reviewed changes

agrawalreetika force-pushed the list-cache-invalidation branch from 2bf4b22 to 0627839 Compare August 9, 2023 08:47

agrawalreetika force-pushed the list-cache-invalidation branch from 0627839 to a45324d Compare December 1, 2023 12:55

steveburnett previously approved these changes Dec 1, 2023

View reviewed changes

tdcmeehan previously approved these changes Jan 26, 2024

View reviewed changes

agrawalreetika force-pushed the list-cache-invalidation branch 2 times, most recently from 180a458 to e361fad Compare February 16, 2024 13:11

agrawalreetika force-pushed the list-cache-invalidation branch 2 times, most recently from 1200de1 to 21bc4e2 Compare February 22, 2024 05:00

agrawalreetika dismissed stale reviews from tdcmeehan, steveburnett, and imjalpreet via 21bc4e2 February 22, 2024 14:49

agrawalreetika force-pushed the list-cache-invalidation branch from 21bc4e2 to 0d6069a Compare February 22, 2024 18:13

agrawalreetika dismissed steveburnett’s stale review via dac10e2 February 26, 2024 18:17

agrawalreetika force-pushed the list-cache-invalidation branch from 89afaf4 to dac10e2 Compare February 26, 2024 18:17

tdcmeehan self-assigned this Feb 26, 2024

agrawalreetika force-pushed the list-cache-invalidation branch 3 times, most recently from 6d5a116 to 2ea3862 Compare February 27, 2024 12:03

tdcmeehan reviewed Feb 27, 2024

View reviewed changes

agrawalreetika force-pushed the list-cache-invalidation branch 6 times, most recently from a6b4e7b to d2f938c Compare February 28, 2024 11:24

agrawalreetika force-pushed the list-cache-invalidation branch 2 times, most recently from 9c55082 to 3097b5e Compare February 28, 2024 19:06

agrawalreetika force-pushed the list-cache-invalidation branch 4 times, most recently from 31d3e88 to 7afa178 Compare March 6, 2024 13:52

Procedure to invalidate directory list cache

004ba4b

agrawalreetika force-pushed the list-cache-invalidation branch from 7afa178 to 004ba4b Compare March 6, 2024 14:01

tdcmeehan approved these changes Mar 6, 2024

View reviewed changes

steveburnett approved these changes Mar 6, 2024

View reviewed changes

tdcmeehan merged commit a4c2639 into prestodb:master Mar 6, 2024
57 checks passed

steveburnett mentioned this pull request Apr 16, 2024

[docs] Fix formatting in Hive Connector Procedures doc #22535

Merged

6 tasks

wanglinsong mentioned this pull request May 1, 2024

Add release notes for 0.287 #22647

Merged

48 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Procedure to invalidate directory list cache #19821

Procedure to invalidate directory list cache #19821

agrawalreetika commented Jun 8, 2023

nmahadevuni commented Jun 9, 2023

agrawalreetika commented Jun 12, 2023

agrawalreetika commented Jun 14, 2023

imjalpreet left a comment

imjalpreet left a comment

tdcmeehan Jun 15, 2023 •

edited

Loading

agrawalreetika Jun 19, 2023

tdcmeehan Jun 22, 2023

mrmaddx2 Jul 13, 2023

github-actions bot commented Dec 1, 2023 •

edited

Loading

steveburnett left a comment

steveburnett commented Jan 29, 2024

tdcmeehan Feb 27, 2024

agrawalreetika commented Feb 28, 2024

tdcmeehan commented Feb 28, 2024

agrawalreetika commented Feb 28, 2024

agrawalreetika commented Mar 6, 2024

steveburnett left a comment

Procedure to invalidate directory list cache #19821

Procedure to invalidate directory list cache #19821

Conversation

agrawalreetika commented Jun 8, 2023

nmahadevuni commented Jun 9, 2023

agrawalreetika commented Jun 12, 2023

agrawalreetika commented Jun 14, 2023

imjalpreet left a comment

Choose a reason for hiding this comment

imjalpreet left a comment

Choose a reason for hiding this comment

tdcmeehan Jun 15, 2023 • edited Loading

Choose a reason for hiding this comment

agrawalreetika Jun 19, 2023

Choose a reason for hiding this comment

tdcmeehan Jun 22, 2023

Choose a reason for hiding this comment

mrmaddx2 Jul 13, 2023

Choose a reason for hiding this comment

github-actions bot commented Dec 1, 2023 • edited Loading

steveburnett left a comment

Choose a reason for hiding this comment

steveburnett commented Jan 29, 2024

tdcmeehan Feb 27, 2024

Choose a reason for hiding this comment

agrawalreetika commented Feb 28, 2024

tdcmeehan commented Feb 28, 2024

agrawalreetika commented Feb 28, 2024

agrawalreetika commented Mar 6, 2024

steveburnett left a comment

Choose a reason for hiding this comment

tdcmeehan Jun 15, 2023 •

edited

Loading

github-actions bot commented Dec 1, 2023 •

edited

Loading