-
Notifications
You must be signed in to change notification settings - Fork 519
feat: add serial commit #5662
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
feat: add serial commit #5662
Conversation
9cfb3fb to
57e3216
Compare
57e3216 to
d1aec0a
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
When a lance dataset is opened, it is bound to a To support strict serial isolation, there are 3 approaches:
Hi @wjones127 @jackye1995 @Xuanwo @majin1102 , please let me know your thoughts, thanks very much! |
|
Let me quote how Iceberg describle serilizable isolation: https://iceberg.apache.org/docs/1.6.0/reliability/
From my understanding, what you're trying to implement in this PR follows a very traditional approach to serializability, which is indeed essential for OLTP workloads. However, for lakehouse formats like Lance, serializable isolation of writes is often sufficient. That said, we still need to address the underlying issue in this scenario. In my opinion, we might need to introduce some form of external locking mechanism beyond the Lance format itself. |
|
Lance operates in SNAPSHOT ISOLATION, so write skew is expected. (maybe we should make that clear in the format spec) I personally like this blog a lot: https://brooker.co.za/blog/2024/12/17/occ-and-isolation.html It describes well why SI is good enough for most cases, and the perf tradeoff. I think that has been kind of proven in Iceberg as well, for write performance, in the end many customers I interacted with switched to SI by default instead of the default SERIALIZABLE behavior. Also, Iceberg has been using the term SERIALIZABLE to describe its single-table guarantee, which is misleading, because after all any table format is just Read-Committed Snapshot Isolation (RCSI) across tables, which is technically weaker than SI. So I think we would need a very strong reason to support SERIALIZABLE. Why do we want to add the support now? Can we just work with SI? |
Partitioned namespace uses a lance table |
|
As we discussed in that PR, this I think is no longer needed? |
Lance transaction is not strict serial isolation. Here is an example of write skew.
In partitioned namespace, we use a special lance table
__manifestas meta store. When commit in cross-partitioned write, we need serial commit in__manifestto guarantee atomic commit.