Constant time attach path lookup and locking to ensure unique file handles #9671
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Just one small note, maybe the ":memory:" should live in this list instead: // NOTE: there is a copy of this in the Postgres' parser grammar (gram.y)
#define DEFAULT_SCHEMA "main"
#define INVALID_SCHEMA ""
#define INVALID_CATALOG ""
#define SYSTEM_CATALOG "system"
#define TEMP_CATALOG "temp"(constants.hpp) |
Mytherin
reviewed
Nov 14, 2023
|
@Mytherin, what about this? |
|
Maybe move it to the define for consistency and we can figure out a cleaner way of doing it later |
…ires the file handle
Mytherin
reviewed
Nov 20, 2023
|
Thanks! |
krlmlr
added a commit
to duckdb/duckdb-r
that referenced
this pull request
Dec 11, 2023
Merge pull request duckdb/duckdb#9671 from taniabogatsch/profile-attach
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses two issues concerning
ATTACH.map<path, name>to look up the path, and we can also use the name to retrieve the matching catalog entry in constant time.map<path, name>to ensure that we never have more than a single file handle to a file.I also noticed that the
":memory:"string crept into our code at many places, so I added a constantDConstants::IN_MEMORY_PATHfor it and replaced all occurrences in thesrcsubdirectories.Here are some timings on my machine for five runs of attaching 30_000 files with 32 threads attaching simultaneously. To run this, the test
test_attach.cppincluded in this PR can be modified to run with higher file and thread counts.