[Kernel]Generalize the actions after commit(like checkpoint) by introducing post commit action to kernel #4115
Conversation
huan233usc
changed the title
kernel/kernel-api/src/main/java/io/delta/kernel/PostCommitAction.java
Outdated
Show resolved
Hide resolved
kernel/kernel-api/src/main/java/io/delta/kernel/PostCommitAction.java
Outdated
Show resolved
Hide resolved
kernel/kernel-api/src/main/java/io/delta/kernel/PostCommitAction.java
Outdated
Show resolved
Hide resolved
kernel/kernel-api/src/main/java/io/delta/kernel/internal/TransactionImpl.java
Outdated
Show resolved
Hide resolved
| import io.delta.kernel.engine.Engine; | ||
| import java.io.IOException; | ||
|
|
||
| public interface PostCommitHook { |
There was a problem hiding this comment.
Should we add more docs here declaring what type of work is considered a PostCommitHook? And how an engine should treat them? i.e. are they required, how do they relate to the commit, etc
Alternatively, maybe more thorough docs for each type?
There was a problem hiding this comment.
Added some doc, PTAL. Thanks
There was a problem hiding this comment.
Also add what sort of operations and latency wise. So that the connector can choose to run it async.
There was a problem hiding this comment.
Updated the documentation to indicate supported operations and latency indication for checkpoint in below section.
kernel/kernel-api/src/main/java/io/delta/kernel/hook/PostCommitHookType.java
Outdated
Show resolved
Hide resolved
kernel/kernel-api/src/main/java/io/delta/kernel/hook/CheckpointHook.java
Outdated
Show resolved
Hide resolved
kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/DeltaTableWritesSuite.scala
Outdated
Show resolved
Hide resolved
| public interface PostCommitHook { | ||
|
|
||
| enum PostCommitHookType { | ||
| // Write a new checkpoint at the version committed by the txn if required. |
There was a problem hiding this comment.
this is maybe a little ambiguous as to when it is present (i.e. is it always present and only sometime checkpoints?)
Maybe something like "Write a new checkpoint at the version committed by the txn. This hook is present when the table is ready for checkpoint according to its configured checkpoint interval"
There was a problem hiding this comment.
You can do /* */ comment for the enum
| import io.delta.kernel.internal.fs.Path; | ||
| import java.io.IOException; | ||
|
|
||
| /** Write a new checkpoint at the version committed by the txn if required. */ |
There was a problem hiding this comment.
| /** Write a new checkpoint at the version committed by the txn if required. */ | |
| /** Write a new checkpoint at the version committed by the txn. */ |
There was a problem hiding this comment.
If this hook is created, it is required
| */ | ||
| public boolean isReadyForCheckpoint() { | ||
| return isReadyForCheckpoint; | ||
| /** @return list of operations to trigger after commit. */ |
There was a problem hiding this comment.
Add more details such as how the connector can choose to run and how it may affect the query performance etc.
There was a problem hiding this comment.
Added a usage section to cover these. "query performance" means the commit latency in this context?
| import io.delta.kernel.engine.Engine; | ||
| import java.io.IOException; | ||
|
|
||
| public interface PostCommitHook { |
There was a problem hiding this comment.
Also add what sort of operations and latency wise. So that the connector can choose to run it async.
Which Delta project/connector is this regarding?
Description
This PR doesn't make any functional changes, but abstract checkpoint into post commit action. This is prepared adding more post commit actions such as CRC write (#4116)
How was this patch tested?
Existing unit test, manual test using delta/kernel/examples/run-kernel-examples.py --use-local
Does this PR introduce any user-facing changes?
No