Skip to content

MongoDB table engine now establishes connection only when it reads data.#20110

Merged
KochetovNicolai merged 2 commits intoClickHouse:masterfrom
vitlibar:mongodb-doesnt-connect-before-read
Feb 12, 2021
Merged

MongoDB table engine now establishes connection only when it reads data.#20110
KochetovNicolai merged 2 commits intoClickHouse:masterfrom
vitlibar:mongodb-doesnt-connect-before-read

Conversation

@vitlibar
Copy link
Copy Markdown
Member

@vitlibar vitlibar commented Feb 5, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category:

  • Bug Fix

Changelog entry:
The MongoDB table engine now establishes connection only when it's going to read data. ATTACH TABLE won't try to connect anymore.

This PR fixes #20055

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Feb 5, 2021
Comment on lines +55 to +57
std::lock_guard lock{connection_mutex};
if (!connection)
connection = std::make_shared<Poco::MongoDB::Connection>(host, port);
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.

I think it is better to release mutex right after connection is initialized.

Copy link
Copy Markdown
Member Author

@vitlibar vitlibar Feb 9, 2021

Choose a reason for hiding this comment

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

It seems there is no point in allowing multiple threads to do the same authentication in parallel.

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.

ok

Comment on lines +55 to +57
std::lock_guard lock{connection_mutex};
if (!connection)
connection = std::make_shared<Poco::MongoDB::Connection>(host, port);
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.

Suggested change
std::lock_guard lock{connection_mutex};
if (!connection)
connection = std::make_shared<Poco::MongoDB::Connection>(host, port);
{
std::lock_guard lock{connection_mutex};
if (!connection)
connection = std::make_shared<Poco::MongoDB::Connection>(host, port);
}


std::shared_ptr<Poco::MongoDB::Connection> connection;
bool authentified = false;
std::mutex connection_mutex; /// Protects the variables `connection` and `authentified`.
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.

I've added the comment.

@KochetovNicolai KochetovNicolai merged commit 2ba503a into ClickHouse:master Feb 12, 2021
robot-clickhouse pushed a commit that referenced this pull request Feb 12, 2021
robot-clickhouse pushed a commit that referenced this pull request Feb 12, 2021
robot-clickhouse pushed a commit that referenced this pull request Feb 12, 2021
alexey-milovidov added a commit that referenced this pull request Feb 12, 2021
Backport #20110 to 20.12: MongoDB table engine now establishes connection only when it reads data.
alexey-milovidov added a commit that referenced this pull request Feb 12, 2021
Backport #20110 to 21.1: MongoDB table engine now establishes connection only when it reads data.
alexey-milovidov added a commit that referenced this pull request Feb 12, 2021
Backport #20110 to 21.2: MongoDB table engine now establishes connection only when it reads data.
robot-clickhouse pushed a commit that referenced this pull request Feb 15, 2021
vitlibar pushed a commit that referenced this pull request Feb 16, 2021
Backport #20110 to 20.8: MongoDB table engine now establishes connection only when it reads data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ch start failed: Caught exception while loading metadata:

3 participants