Skip to content
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

Vacuum isuses for ADLS #647

Closed
Blajda opened this issue Jun 19, 2022 · 3 comments
Closed

Vacuum isuses for ADLS #647

Blajda opened this issue Jun 19, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@Blajda
Copy link
Collaborator

Blajda commented Jun 19, 2022

Environment

Delta-rs version: git 649fdce


Bug

What happened:
Calling table.vacuum with a ADLS backed table results in a panic. Working around the panic results in no files being picked up.

thread 'vacuum::test_adls_vacuum' panicked at 'byte index 80 is out of bounds of adls2://mystorage@mystorageacct.dfs.core.windows.net/mystorage/mytable', library/core/src/str/mod.rs:107:9

(Ignore the Index number. Modified the paths of my accounts)

Adding a / to the end of the table URI does not work since some normalization is performed during table load but would not matter due to the below issues.

More details:

I removed the + 1 on this line to check if it was the main issue but encounter other issues.

list_obj for ADLS may have two issues:

  1. When ObjectMeta is created path from the callee of list_obj is used and not the actual paths from the API.
  2. Currently recursive is set to false. Vacuum only calls list_obj for the table's URI hence only _delta_log and partition directories are being returned. This might be an issue with how vacuum is implemented.

See: https://github.com/delta-io/delta-rs/blob/main/rust/src/storage/azure/mod.rs#L369-L375

@Blajda Blajda added the bug Something isn't working label Jun 19, 2022
@Blajda
Copy link
Collaborator Author

Blajda commented Jun 19, 2022

Hi @roeap, looking for you analysis on this. If (2.) should be handled from the vacuum side I can take that work on.

@roeap
Copy link
Collaborator

roeap commented Jun 22, 2022

@Blajda - thanks for the report! will take a look and see what needs to be done ...

@Blajda
Copy link
Collaborator Author

Blajda commented Jul 12, 2022

Closing since #683 is merged and the vacuum tests passed.

@Blajda Blajda closed this as completed Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants