feat: upgrade DataFusion, Arrow, PyO3, ObjectStore #2594
Conversation
github-actions
bot
added
enhancement
There was a problem hiding this comment.
Most of this was ported from the now deleted gcs_wrapper.rs
| impl Drop for ObjectWriter { | ||
| fn drop(&mut self) { | ||
| // If there is a multipart upload started but not finished, we should abort it. | ||
| if matches!(self.state, UploadState::InProgress { .. }) { | ||
| // Take ownership of the state. | ||
| let state = std::mem::replace(&mut self.state, UploadState::Done); | ||
| if let UploadState::InProgress { mut upload, .. } = state { | ||
| tokio::task::spawn(async move { | ||
| let _ = upload.abort().await; | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This is what ensures partial uploads are cleaned up now. MultipartId is no longer exposed in the object-store API.
wjones127
force-pushed
the
feat/upgrade-datafusion
branch
from
8d805cf to
66dec81
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2594 +/- ##
==========================================
+ Coverage 79.97% 80.22% +0.24%
==========================================
Files 212 211 -1
Lines 61556 61418 -138
Branches 61556 61418 -138
==========================================
+ Hits 49228 49270 +42
+ Misses 9402 9217 -185
- Partials 2926 2931 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
rust/lance-datagen/src/generator.rs
Outdated
| // Native types, which don't support Distribution. | ||
| // IntervalUnit::DayTime => rand::<IntervalDayTimeType>(), | ||
| // IntervalUnit::MonthDayNano => rand::<IntervalMonthDayNanoType>(), | ||
| IntervalUnit::DayTime | IntervalUnit::MonthDayNano => todo!(), |
There was a problem hiding this comment.
I added some utils to do this,
post it here if you would like to use it
https://github.com/lancedb/lance/pull/2589/files#diff-05a6cb2a51514a3883006392e5a3b6990e908be79d42414de787ba5c5b623c87R600
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
This looks quite exhausting. Thanks for the cleanup
| DataType::Interval(unit) => match unit { | ||
| IntervalUnit::DayTime => rand::<IntervalDayTimeType>(), | ||
| IntervalUnit::MonthDayNano => rand::<IntervalMonthDayNanoType>(), | ||
| IntervalUnit::YearMonth => rand::<IntervalYearMonthType>(), | ||
| }, | ||
| DataType::Interval(unit) => rand_interval(*unit), |
There was a problem hiding this comment.
I think they changed the underlying native type to a struct rather than i128/i64. Thus Distribution was no longer implemented.
| pub async fn cleanup_partial_writes( | ||
| store: &ObjectStore, | ||
| base_path: &Path, | ||
| objects: impl IntoIterator<Item = (&Path, &String)>, | ||
| ) -> Result<()> { |
There was a problem hiding this comment.
Why get rid of this? Was it no longer working? Or no longer needed?
There was a problem hiding this comment.
object_store no longer exposes the multipart ids. So now users can't collect them and pass them into this function.
Instead, we automatically cleanup on Drop for ObjectWriter. In case of crashes, users should rely on setting a lifecycle rule to delete old incomplete writes. This is already recommended in our docs.
4 participants
BREAKING CHANGE:
MultipartId,cleanup_partial_writes. Uploads are now automatically cleaned up.FragmentWriteProgressno longer has anymultipart_idargument.CommitHandlerandManifestWriterAPIs now take a LanceObjectStorerather thanobject_store::ObjectStore.New features:
Connection resetnetwork errors.