feat: upgrade DataFusion, Arrow, PyO3, ObjectStore #2594

wjones127 · 2024-07-12T22:28:02Z

BREAKING CHANGE:

Removed access to MultipartId, cleanup_partial_writes. Uploads are now automatically cleaned up.
- FragmentWriteProgress no longer has any multipart_id argument.
CommitHandler and ManifestWriter APIs now take a Lance ObjectStore rather than object_store::ObjectStore.

New features:

For S3 and Azure (these features were already implemented for GCS):
- Max object size is now 2.5 TB, up from 50GB.
- Retries are now performed for Connection reset network errors.
- Reduced number of IOPS for writing small objects (<5MB)

wjones127 · 2024-07-13T02:57:39Z

rust/lance-io/src/object_writer.rs

Most of this was ported from the now deleted gcs_wrapper.rs

wjones127 · 2024-07-13T02:59:19Z

rust/lance-io/src/object_writer.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;
+                });
+            }
+        }
    }
 }


This is what ensures partial uploads are cleaned up now. MultipartId is no longer exposed in the object-store API.

codecov-commenter · 2024-07-13T03:41:18Z

Codecov Report

Attention: Patch coverage is 53.81679% with 242 lines in your changes missing coverage. Please review.

Project coverage is 80.22%. Comparing base (8cab512) to head (895d2ee).
Report is 1 commits behind head on main.

Files	Patch %	Lines
rust/lance-io/src/object_writer.rs	67.93%	71 Missing and 13 partials ⚠️
rust/lance-datagen/src/generator.rs	5.12%	37 Missing ⚠️
rust/lance-index/src/scalar/btree.rs	4.00%	23 Missing and 1 partial ⚠️
rust/lance/src/io/exec/planner.rs	34.48%	18 Missing and 1 partial ⚠️
rust/lance/src/io/exec/scalar_index.rs	15.38%	11 Missing ⚠️
rust/lance/src/io/exec/knn.rs	43.75%	9 Missing ⚠️
rust/lance-index/src/scalar/expression.rs	0.00%	6 Missing ⚠️
rust/lance-io/src/object_store.rs	72.22%	3 Missing and 2 partials ⚠️
rust/lance/src/io/exec/utils.rs	0.00%	5 Missing ⚠️
rust/lance-datafusion/src/exec.rs	0.00%	4 Missing ⚠️
... and 15 more

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

Flag	Coverage Δ
unittests	`80.22% <53.81%> (+0.24%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Cargo.toml

python/python/lance/cleanup.py

BubbleCal · 2024-07-13T04:44:10Z

rust/lance-datagen/src/generator.rs

+                // Native types, which don't support Distribution.
+                // IntervalUnit::DayTime => rand::<IntervalDayTimeType>(),
+                // IntervalUnit::MonthDayNano => rand::<IntervalMonthDayNanoType>(),
+                IntervalUnit::DayTime | IntervalUnit::MonthDayNano => todo!(),


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

rust/lance-io/src/object_store.rs

westonpace

This looks quite exhausting. Thanks for the cleanup

westonpace · 2024-07-16T19:26:00Z

rust/lance-datagen/src/generator.rs

-            DataType::Interval(unit) => match unit {
-                IntervalUnit::DayTime => rand::<IntervalDayTimeType>(),
-                IntervalUnit::MonthDayNano => rand::<IntervalMonthDayNanoType>(),
-                IntervalUnit::YearMonth => rand::<IntervalYearMonthType>(),
-            },
+            DataType::Interval(unit) => rand_interval(*unit),


Why the change here?

I think they changed the underlying native type to a struct rather than i128/i64. Thus Distribution was no longer implemented.

apache/arrow-rs#5769

westonpace · 2024-07-16T19:29:47Z

rust/lance/src/dataset/cleanup.rs

-pub async fn cleanup_partial_writes(
-    store: &ObjectStore,
-    base_path: &Path,
-    objects: impl IntoIterator<Item = (&Path, &String)>,
-) -> Result<()> {


Why get rid of this? Was it no longer working? Or no longer needed?

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.

github-actions bot added enhancement New feature or request breaking-change python labels Jul 12, 2024

wjones127 commented Jul 13, 2024

View reviewed changes

rust/lance-io/src/object_writer.rs

Copy link

Contributor Author

wjones127 Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this was ported from the now deleted gcs_wrapper.rs

wjones127 commented Jul 13, 2024

View reviewed changes

wjones127 added 4 commits July 12, 2024 20:02

fix lance-io

ee37e85

fix all errors and tests

395dfc0

upgrade python

5b5926c

fix rebase errors

66dec81

wjones127 force-pushed the feat/upgrade-datafusion branch from 8d805cf to 66dec81 Compare July 13, 2024 03:12

version policy

8d7b61d

debug ci job

1770d83

wjones127 marked this pull request as ready for review July 13, 2024 04:25

wjones127 requested review from eddyxu, westonpace and BubbleCal July 13, 2024 04:25

BubbleCal mentioned this pull request Jul 13, 2024

chore: upgrade arrow to v0.52.1 #2589

Closed

BubbleCal reviewed Jul 13, 2024

View reviewed changes

rust/lance-io/src/object_store.rs Show resolved Hide resolved

wjones127 added 2 commits July 15, 2024 10:40

upgrade to 40

0f1dded

fix planner bug

84967b2

wjones127 requested a review from BubbleCal July 15, 2024 20:22

westonpace approved these changes Jul 16, 2024

View reviewed changes

Merge branch 'main' into feat/upgrade-datafusion

895d2ee

wjones127 merged commit 30af1d8 into lancedb:main Jul 16, 2024
22 checks passed

wjones127 deleted the feat/upgrade-datafusion branch July 16, 2024 20:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade DataFusion, Arrow, PyO3, ObjectStore #2594

feat: upgrade DataFusion, Arrow, PyO3, ObjectStore #2594

wjones127 commented Jul 12, 2024 •

edited

Loading

wjones127 Jul 13, 2024

wjones127 Jul 13, 2024

codecov-commenter commented Jul 13, 2024 •

edited

Loading

BubbleCal Jul 13, 2024

wjones127 Jul 15, 2024

westonpace left a comment

westonpace Jul 16, 2024

wjones127 Jul 16, 2024

westonpace Jul 16, 2024

wjones127 Jul 16, 2024

feat: upgrade DataFusion, Arrow, PyO3, ObjectStore #2594

feat: upgrade DataFusion, Arrow, PyO3, ObjectStore #2594

Conversation

wjones127 commented Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 13, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonpace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjones127 commented Jul 12, 2024 •

edited

Loading

codecov-commenter commented Jul 13, 2024 •

edited

Loading