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

bottomless2: perform store job #1405

Merged
merged 4 commits into from
May 28, 2024
Merged

bottomless2: perform store job #1405

merged 4 commits into from
May 28, 2024

Conversation

MarinPostma
Copy link
Collaborator

  • implement scheduler
  • move bottomless2 to libsql-wal
  • add async fs methods
  • implement perform job

@MarinPostma MarinPostma force-pushed the perform-bottomless-job branch 3 times, most recently from 58dce40 to 71283a2 Compare May 24, 2024 13:07
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just quieting warnings

@@ -19,3 +19,12 @@ thiserror = "1.0.58"
tracing = "0.1.40"
walkdir = "2.5.0"
zerocopy = { version = "0.7.32", features = ["derive", "alloc"] }
tokio = { version = "1", features = ["full"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need all the features?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably not, but we're compiling them anyway. I'll trim later.

@@ -19,3 +19,12 @@ thiserror = "1.0.58"
tracing = "0.1.40"
walkdir = "2.5.0"
zerocopy = { version = "0.7.32", features = ["derive", "alloc"] }
tokio = { version = "1", features = ["full"] }
tempfile = "3.10.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

dev dep?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nope, it's being used

let mut written = 0;

while written != buf.len() {
written += nix::sys::uio::pwrite(self, &buf[written..], offset as _)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

dont you need to write at offset + written?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm yeah, I need to write a test that catches that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was unable to write a test to verify that, no matter how big the payload, it's writing everything in one go...

// todo: create an async counterpart
fn tempfile(&self) -> io::Result<Self::TempFile>;
fn now(&self) -> DateTime<Utc>;
fn uuid(&self) -> Uuid;
Copy link
Contributor

Choose a reason for hiding this comment

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

uuid is part of the io system 🤣

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's randomness

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

randomness is an input

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually I'll probably just expose a range 🤔

@MarinPostma MarinPostma force-pushed the perform-bottomless-job branch 2 times, most recently from 0a364f0 to 4b47e80 Compare May 27, 2024 10:10
@MarinPostma MarinPostma marked this pull request as ready for review May 28, 2024 17:02
while let Some((page_no_bytes, offset)) = pages.next() {
let page_no = u32::from_be_bytes(page_no_bytes.try_into().unwrap());
let (b, ret) = segment.read_frame_offset_async(offset as _, buffer).await;
ret.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

whats this unwrap for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Uh yeah, I should return an io error here, I'll do in a follow up

}

fn uuid(&self) -> Uuid {
dbg!();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dbg!();

@@ -43,7 +52,7 @@ impl<FS: FileSystem> WalManager for LibsqlWalManager<FS> {
.registry
.clone()
.open(db_path.as_ref())
.map_err(Into::into)?;
.map_err(|e| dbg!(e.into()))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove this?

@LucioFranco LucioFranco added this pull request to the merge queue May 28, 2024
Merged via the queue into main with commit 3f330d6 May 28, 2024
17 checks passed
@LucioFranco LucioFranco deleted the perform-bottomless-job branch May 28, 2024 19:26
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.

None yet

2 participants