Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check table and view existence in Memory connector #11453

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Mar 14, 2022

Description

Check table and view existence in Memory connector

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

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

@cla-bot cla-bot bot added the cla-signed label Mar 14, 2022
@ebyhr ebyhr requested a review from findepi March 14, 2022 00:00
@findepi findepi requested review from kokosing, hashhar and wendigo and removed request for findepi March 14, 2022 08:34
@findepi
Copy link
Member

findepi commented Mar 14, 2022

(x) No release notes entries required.

True if we merge this in 374, otherwise this is a bug fix.

#11421 (comment)

Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

LGTM % thread-safety (it was pre-existing anyway so I'm ok with current impl too).

verify(schemas.remove(schemaName));
}

private boolean isSchemaEmpty(String schemaName)
{
if (tables.values().stream()
Copy link
Member

Choose a reason for hiding this comment

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

Is this thread-safe? It's backed by a normal HashMap and there is no synchronisation.

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be. https://docs.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html

First, it is not possible for two invocations of synchronized methods on the same object to interleave. When one thread is executing a synchronized method for an object, all other threads that invoke synchronized methods for the same object block (suspend execution) until the first thread is done with the object.

@ebyhr ebyhr merged commit d8e111c into trinodb:master Mar 15, 2022
@ebyhr ebyhr deleted the ebi/memory-verify-table-views branch March 15, 2022 02:40
@github-actions github-actions bot added this to the 374 milestone Mar 15, 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.

3 participants