Support ALTER TABLE EXECUTE in event driven scheduler#14756
Support ALTER TABLE EXECUTE in event driven scheduler#14756losipiuk merged 5 commits intotrinodb:masterfrom
Conversation
There was a problem hiding this comment.
Another approach would be to change cleanExtraOutputFiles, so it strips location from path stored in filesToKeep. Theoretically, it may prevent us from deleting something we should delete, but it is a very slim chance of that. No chance really as filenames contain UUID.
@findepi do you think I should drop the change in the test and modify prod code instead?
There was a problem hiding this comment.
Another approach would be to change cleanExtraOutputFiles, so it strips location from path stored in
filesToKeep.
That's what @alexjo2144 did for orphan removal.
It's because paths can be compared reliably only if they are "canonical". But the concept of canonical paths is not very well defined, is it?
Sounds reasonable to me.
losipiuk
force-pushed
the
lo/iceberg-with-retries-test
branch
from
6509a00 to
42a70ae
Compare
|
"Use just filename when cleaning up extra output files" LGTM cc @alexjo2144 |
|
One test hang: thread dump: https://gist.github.com/losipiuk/4cb58f0bc5b692cefe85e0cfdd01a61b |
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
losipiuk
force-pushed
the
lo/iceberg-with-retries-test
branch
from
eebe4a7 to
855fb4c
Compare
Previously when determining if file should be deleted at the end of DML operation because it comes from failed task was based on full path. It does not work great with case insensitive filesystems. This PR changes logic to just look at filename ignoring the path, it is still ok as filenames have UUID weaved in hence are unique (note: even on conflit we would not loose any data, worst thing that can happen that some garbage will be left behing, and cleaned up later).
This increases test coverage. So far we did not have coverage for MERGE and ALTER TABLE EXECUTE
losipiuk
force-pushed
the
lo/iceberg-with-retries-test
branch
from
855fb4c to
b2b10b3
Compare
4 participants
(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:
Note: Technically nothing changed as fixes are related to even driven FTE scheduler which was not released yet