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

Add support for Snowflake Stages and SNOWFLAKE_FULL encryption #24

Merged
merged 15 commits into from
Oct 15, 2024

Conversation

andrebsguedes
Copy link
Member

@andrebsguedes andrebsguedes commented Sep 19, 2024

This PR consolidates all changes on the Rust side for supporting Snowflake internal stages as a first class store.
It also restructures much of the code (with many misc changes) to ensure the encryption functionality could be introduced in a generic way.

Copy link

@alexrenz alexrenz left a comment

Choose a reason for hiding this comment

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

Started reading. Looks really cool, but definitely out of my zone for now (but I would really like to change that!). I have a few comments, but those are mostly curious questions

src/encryption.rs Show resolved Hide resolved
src/encryption.rs Show resolved Hide resolved
src/snowflake/client.rs Show resolved Hide resolved
let n_files = 20_000;
let concurrency = 512;

// let t0 = std::time::Instant::now();

Choose a reason for hiding this comment

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

just commenting in case this was left unintentionally

src/lib.rs Show resolved Hide resolved
};
}

metric_const!(get_attempt_duration);
Copy link
Member

Choose a reason for hiding this comment

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

how can we read these metrics?

Copy link
Member Author

@andrebsguedes andrebsguedes Oct 15, 2024

Choose a reason for hiding this comment

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

We can't read them currently as they are not getting plumbed all the way to Julia, I am afraid the plumbing will have to come in a later PR.

src/encryption.rs Show resolved Hide resolved

struct Buffer {
buf: Vec<u8>,
pos: usize,
Copy link
Member

Choose a reason for hiding this comment

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

From call: both of pos and filled are monotonically increasing and chase each other.

}
}
State::Filling => {
if this.inbuf.filled().len() <= extract_tag_len {
Copy link
Member

Choose a reason for hiding this comment

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

from call: to be able to find out whether the buffered input is tag or sth else

@adnan-alhomssi
Copy link
Member

A ToDo for one day is to test the encrypted put/get using randomized input/chunk size as a stress test to exercise all state machine transitions inside CrypterReader and CrypterWriter

@andrebsguedes
Copy link
Member Author

I managed to write the randomized test for CrypterWriter and CrypterReader and add the requested clarifications.

@andrebsguedes andrebsguedes merged commit 924ac1d into main Oct 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants