Skip to content

Share nothing when the table snapshots cache is disabled#13086

Merged
ebyhr merged 1 commit intotrinodb:masterfrom
findinpath:delta-lake-cache-share-nothing-when-disabled
Jul 6, 2022
Merged

Share nothing when the table snapshots cache is disabled#13086
ebyhr merged 1 commit intotrinodb:masterfrom
findinpath:delta-lake-cache-share-nothing-when-disabled

Conversation

@findinpath
Copy link
Copy Markdown
Contributor

@findinpath findinpath commented Jul 5, 2022

Description

When using the setting for the Delta Lake connector:

delta.metadata.cache-ttl=0s

the Trino server wouldn't start up correctly anymore and would throw the exception:

ERROR main  io.trino.server.Server Unable to create injector, see the following errors:\
\
1) [Guice/ErrorInjectingConstructor]: IllegalStateException: Even when cache is disabled, the loads are synchronized and both load results and failures are shared between threads. This is rarely desired, thus builder caller is expected to either opt-in into this behavior with shareResultsAndFailuresEvenIfDisabled(), or choose not to share results (and failures) between concurrent invocations with shareNothingWhenDisabled().\
 at TransactionLogAccess.<init>(TransactionLogAccess.java:105)\
 at GeneratedMethodAccessor150.invoke(Unknown Source)\
 at DeltaLakeMetadataFactory.<init>(DeltaLakeMetadataFactory.java:74)\
   \\_ for 3rd parameter transactionLogAccess\
 at GeneratedMethodAccessor150.invoke(Unknown Source)\
 at DeltaLakeTransactionManager.<init>(DeltaLakeTransactionManager.java:38)\
   \\_ for 1st parameter metadataFactory\
 at GeneratedMethodAccessor150.invoke(Unknown Source)\
 at DeltaLakeModule.createMetastoreGetter(DeltaLakeModule.java:174)\
   \\_ for 1st parameter transactionManager\
 at DeltaLakeModule.createMetastoreGetter(DeltaLakeModule.java:174)\
 at DeltaLakeSplitManager.<init>(DeltaLakeSplitManager.java:80)\
   \\_ for 2nd parameter metastoreProvider\
 while locating DeltaLakeSplitManager\
 at GeneratedMethodAccessor150.invoke(Unknown Source)\
 while locating ConnectorSplitManager\

Add specific handling to cope with this runtime exception so that, similarly to the other caches from Trino, nothing is shared when the table snapshots cache is disabled.

Is this change a fix, improvement, new feature, refactoring, or other?

Fix

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Delta Lake connector

How would you describe this change to a non-technical end user or system administrator?

Specified behavior NOOP for the situation when the table snapshots cache is disabled.

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Delta Lake
* Bugfix: Delta Lake connector starts now also when the internal table snapshots cache is disabled.

@cla-bot cla-bot bot added the cla-signed label Jul 5, 2022
@findinpath findinpath requested review from ebyhr, losipiuk and mdesmet July 5, 2022 13:09
@findinpath findinpath self-assigned this Jul 5, 2022
@findinpath findinpath force-pushed the delta-lake-cache-share-nothing-when-disabled branch from d365be2 to 7f19b83 Compare July 6, 2022 08:40
@findinpath findinpath force-pushed the delta-lake-cache-share-nothing-when-disabled branch from 7f19b83 to e2fe598 Compare July 6, 2022 08:41
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Jul 6, 2022

No release notes entries required.

I think we can add an entry to release note.

@ebyhr ebyhr merged commit bd33388 into trinodb:master Jul 6, 2022
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Jul 6, 2022

Merged, thanks!

@ebyhr ebyhr mentioned this pull request Jul 6, 2022
@github-actions github-actions bot added this to the 389 milestone Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants