opendal icon indicating copy to clipboard operation
opendal copied to clipboard

feat(services/azdls): list start from

Open alexwilcoxson-rel opened this issue 1 year ago • 15 comments

Which issue does this PR close?

Closes #.

Rationale for this change

This changes proposes implementation of the start_after workaround used by Hadoop to improve listing in ADLS from a particular point.

This is important for table formats like Delta Lake that make use of list_with_offset in object store to get the latest version of a table.

In my testing listing directories with > 10k records similar to what Delta Lake would do improves times from seconds to milliseconds (~3seconds to 60ms).

Also this adds SAS token support which could be split into its own PR but needed it for testing against my company's ADLS accounts.

What changes are included in this PR?

Changes to azdls module to include list with start_after functionality. This includes a module crc64 which computes crc for use in the continuation token generated for start after

Are there any user-facing changes?

Yes, start_after will function for azdls, and SAS token support.


@Xuanwo I'm opening this as a draft to get initial feedback. This is not directly supported by Microsoft, or I should say documented. In pursuit of them adding this capability to the azblob endpoints, they were the ones that directed me to Hadoop example, so I have confidence in it but could see if its something they would officially support/document.

One caveat as well is I believe (per the hadoop code) the change is will only work if hierarchical namespace (xns) is enabled which isn't something I believe we can tell when the list request is being made. It could be the user has to opt in and inform if xns is enabled. Hadoop code does another request to figure that out dynamically which I'd personally want to avoid.

alexwilcoxson-rel avatar Oct 24 '24 20:10 alexwilcoxson-rel

hey @Xuanwo as I was refactoring the code I wrote a test against one of our actual storage accounts. I then executed the list call with recursive = true as the object_store integration does. However when I do that list result from azure contains everything.

I'm trying to figure out how recursive could be impacting the azdls list but I don't see it used by azdls service.

EDIT: ah I see the CompleteLayer, looking into this.

alexwilcoxson-rel avatar Oct 28 '24 20:10 alexwilcoxson-rel

Ok so I found the FlatLister does not retain the OpList args.

Adding modifying FlatLister to take in the original args and pass them to the inner lister seems to work for this case, but I'm not sure how recursive listing and start_after should interact for all edge cases.

The underlying Azure REST API does have a recursive parameter, but when actually using that:

  1. the recursive listing performance in general is not great on azdls
  2. start_after continuation token causes 500 when recursive = true so I am hesitant to enable it

alexwilcoxson-rel avatar Oct 29 '24 01:10 alexwilcoxson-rel

Thank you @alexwilcoxson-rel for catching this.

Adding modifying FlatLister to take in the original args and pass them to the inner lister seems to work for this case, but I'm not sure how recursive listing and start_after should interact for all edge cases.

It should be fine as long as we handle the capability correctly.

The underlying Azure REST API does have a recursive parameter, but when actually using that:

  1. the recursive listing performance in general is not great on azdls
  2. start_after continuation token causes 500 when recursive = true so I am hesitant to enable it

That seems to be a problem. Would you like to raise a separate issue for this? We can track it and switch to azdls's own recursive when available.

Xuanwo avatar Oct 29 '24 02:10 Xuanwo

The underlying Azure REST API does have a recursive parameter, but when actually using that:

  1. the recursive listing performance in general is not great on azdls
  2. start_after continuation token causes 500 when recursive = true so I am hesitant to enable it

That seems to be a problem. Would you like to raise a separate issue for this? We can track it and switch to azdls's own recursive when available.

alexwilcoxson-rel avatar Oct 30 '24 00:10 alexwilcoxson-rel

This feature is undocumented, so it's best not to enable it by default. How about adding a new flag called enable_list_with_start_after, where we set the capability list_with_start_after to true?

  • [x] gate behind feature

alexwilcoxson-rel avatar Oct 30 '24 00:10 alexwilcoxson-rel

Hey @Xuanwo just letting you know I probably won't get back to this until later in the week. I will share though that I am using this in our fork of version ~0.48 I think. We're using it along size the base object_store azure store (since those have all the put support for delta), and it has gone well!

Looking forward to getting this in along with the put if-not-match changes that are in progress as well!

alexwilcoxson-rel avatar Nov 04 '24 19:11 alexwilcoxson-rel

Hi, @alexwilcoxson-rel, do we have a status change here? I'm open to get this feature merged under a new config called enable_list_with_start_after.

Xuanwo avatar Apr 07 '25 11:04 Xuanwo

Hey @Xuanwo sorry for the delay on this, I have some cycles to try to get it across the line

alexwilcoxson-rel avatar May 05 '25 17:05 alexwilcoxson-rel

This feature is undocumented, so it's best not to enable it by default. How about adding a new flag called enable_list_with_start_after, where we set the capability list_with_start_after to true?

done

alexwilcoxson-rel avatar May 05 '25 18:05 alexwilcoxson-rel

In testing this with deltalake table loads, i see the object_store integration is doing a stat per list entry

is there a way around this if the list response contains the needed metadata? i notice the metadata key stuff was removed (for the better :smile: )

alexwilcoxson-rel avatar May 05 '25 19:05 alexwilcoxson-rel

sas_token support has been added in https://github.com/apache/opendal/pull/6205

Xuanwo avatar May 21 '25 09:05 Xuanwo

In testing this with deltalake table loads, i see the object_store integration is doing a stat per list entry

is there a way around this if the list response contains the needed metadata? i notice the metadata key stuff was removed (for the better 😄 )

@Xuanwo can you provide input here please?

alexwilcoxson-rel avatar May 27 '25 15:05 alexwilcoxson-rel

In testing this with deltalake table loads, i see the object_store integration is doing a stat per list entry

is there a way around this if the list response contains the needed metadata? i notice the metadata key stuff was removed (for the better 😄 )

I think it's fine for us to simply remove the stat from the list, since object_store doesn't do that anyway. We always generate the same metadata as object_store does.

Xuanwo avatar May 28 '25 03:05 Xuanwo

@Xuanwo if your okay with latest changes, I can create follow up issues for

  • [ ] recursive investigation on azdls
  • [ ] object_store stat removal during listing

alexwilcoxson-rel avatar May 28 '25 20:05 alexwilcoxson-rel

if your okay with latest changes, I can create follow up issues for

Perfect! Most changes looks good to me. We only need to remove the FlatLister changes.

Xuanwo avatar May 29 '25 04:05 Xuanwo

Sorry I have fallen behind on this and have been working on other initiatives at work. However there is progress in official list_with_offset/startFrom support. See my update here: https://github.com/apache/arrow-rs-object-store/issues/461#issuecomment-3458389196

alexwilcoxson-rel avatar Oct 31 '25 21:10 alexwilcoxson-rel