-
Notifications
You must be signed in to change notification settings - Fork 42
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
Chore: Delta tables should not require an empty directory #3160
Comments
This is a pretty ambiguous case, though it doesn't seem it: on the surface, the thing you expect, should just work or be able to work, but if you dig in a bit further it becomes really hard to make the semantics come out reasonable. There are a couple of high level facts and assumptions here:
There's already some degree of awkward semantics around COPY TO, depending if the target of the operation exists or not. If the target doesn't exist, then everything should create the resources. If the target data source exists, then the operation should overwrite existing data. This is a pretty reasonable high level target, but it's hard to actually implement this:
None of these are exactly the case we have here. I think we might be able to relax the requirement for the empty directory and just create the directory, but if there is a directory that exists and is empty, should we write into it? What about data that isn't delta-related data? It's a lot harder to say... |
I hear you. In service of trying to keep this simpler, I'm trying to paint with a broad brush, while also realizing that we're not capturing some nuance here. Taking that approach, I would say:
That said, I think we should consider an "overwrite" or "if not exists" argument with COPY TO (for all file types), and we may want to consider setting the default there to raising an error instead of overwriting. |
This isn't really possible as an atomic operation. (that's sort of the crux of the problem). It's not (particularly) possible in the cloud, but our tools aren't as clear there.
This is fine, but "will write into a directory with files in it but not if the directory exists and is empty." If, however, you have two threads or operations running at the same time (locally) what do you do? The "most correct answer" is to not put tables in the file system in the addressable path that a user tells us to, and rather write the files to a directory with a UUID as a name and then map table names to tables at the catalog layer. This also means that "deletes" can be a metadata operation (delete from the catalog) with cleanup deletes happening later. (I think we do something similar for native storage, or certainly can), but when we're writing files out for people it's a different story. The multi threaded experience is worse for writing (single) local files than it is for object store files: locally we'll end up corrupting the file (on most filesystems, though perhaps not (always) on windows, definitely a silver lining) whereas on object storage only one operation can succeed. I am in favor of "if not exists" and "overwrite" being options. |
Could you explain a bit more about atomicity and multi-threading here? I guess, more specifically, how does this case differ from, say, two people trying to write the same large Parquet file at the same time?
I don't think it should do this either. I would think that COPY TO would fail if the directory exists at all.
Could you say a bit more about this? It feels kind of like an additional layer of log file, on top of the log file that's already built in to a Delta table. Where would the mapping live? For clarity's sake, could you sketch out what this would look like for the initial write, and then for any subsequent operations? I think you understand the internals here much better than I do, so ELI5 here would be really helpful to get aligned. |
Delta saves data in multiple files (i.e. only the changes/"deltas") and has a single metadata file that maps where data are and tells the client how to assemble a single table from a bunch of parquet files. This means that individual write operations can be very small (write the data to the parquet file and then the metadata file as the commit) which means that multiple writers can write to a single delta table without coordination (because of the all-or-nothing promise of the metadata write to a single file,) you get full MVCC semantics without coordination, which is pretty nifty. There might be edge cases about two writers trying to create a single table (haven't read the details around the spec). Filesystems don't (all) have the same kinds of atomic write promises that object storage has (in change for other promises,) which means "external local filesystem tables (which might have multiple writers)" and "native storage on local file systems with a cluster of worker nodes," are not (to my mind, at the moment) definitely safe (without further testing and investigation). |
I can see how this would apply to INSERTs (I imagine that when inserting into Delta, we're actually creating a new file with the changes), but does it apply to COPY TO as well (in a way that's different from the other COPY TO formats)? Could you also explain why creating the directory changes the outcome here? Is it that it takes two steps instead of one (creating the directory and then creating the table)? |
Exactly. Earlier you said we should have the following semantics:
On object storage, you don't actually have to delete anything to write a new table. and then
These can't both exist at the same time. The first rule isn't really possible to implement locally because the second writer would start deleting things that the first writer. You can implement this in object storage, because I think we just don't delete things, until a later vacuum operation. The second rule is possible to implement (locally and in object storage) but does mean that you'd have to go in and manually clean up after failed operations, which is dodgy, and I think isn't super useful. To be clear, the current behavior is weird as hell, and I think we should change it. The problem is but to what. I think we try and make a directory if possible, and maybe just proceed with directories that have files in them already (there are unlikely to be name collisions, and this is essentially what happens in object storage. |
Ah, thanks for explaining. That all makes sense. What you propose seems reasonable. I think that creating the directory if it doesn't exist, and otherwise writing to the existing directory seems OK. It would help if we log what we're doing (creating a new directory vs writing to an existing directory) to make it less surprising for people. |
Where do you think we should be logging or noting this (and do we have other logging like this that people might expected to look at? |
I was imagining this would happen in the same place where we output the number of rows written, or that the operation succeeded or failed. E.g. currently this: create table abc as select 1 outputs |
We can't do this for cloud as prefixes aren't explicitly created (e.g. objects with a prefix are created.)
This is only an artifact of the shell itself, and I believe not something that is happening lower in the database. Plumbing the message from the data source up to the shell is a pretty big lift, and it doesn't quite answer what we do in the bindings. |
Could we check to see whether objects with the prefix exist, and message accordingly?
I'm not quite following here. Could you explain with a bit more detail? Do you have any suggestions for another approach? |
From the perspective of user's there isn't really anything/anywhere to log during successful operations, or at least not consistently given the different ways glaredb is used/called, because conceptually it's still client/server and the results of a successful operation are a [conceptual] cursor of results.
Nope. I don't think "logging on success" is a thing we can (or should) do. We can definitely log normally, user's will (mostly) not see this. |
Currently, when I call something like: con.sql("copy (select 1) to 'hello.csv'").show() I see a table with the message:
How does this differ from logging if a table is created? |
These results are rendered here and while we could have a different message, it'd be a bit of a bodge. Also, I think it would be good if this operation (like all operations that write data) return a message with the total number of rows written. |
I think that having generally useful information about the write would be good, including the number of rows and whether it was successful. |
Description
Currently, when copying to a Delta table, a user needs to create an empty directory, and then pass that empty directory in to the COPY TO command, e.g.:
This will raise an error if the directory does not exist, and will also raise an error if the directory is not empty. Instead, we should create the directory for the user before copying the Delta table to it. This removes ambiguity for the function and is also more consistent with the behavior of other COPY TO formats.
When copying to a CSV or Parquet, the file is created directly. If a Delta table is comprised of a directory with some files and logs in it, it makes sense that we would create the entire Delta table, including the directory, instead of filling an existing directory with the contents of a Delta table.
Instead, when copying to a delta table, it should look like this:
The text was updated successfully, but these errors were encountered: