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

[WIP] Use fetch_max instead of bump_atomic_lsn #1233

Closed
wants to merge 1 commit into from
Closed

[WIP] Use fetch_max instead of bump_atomic_lsn #1233

wants to merge 1 commit into from

Conversation

Yam76
Copy link

@Yam76 Yam76 commented Jan 1, 2021

The relevant part of issue rust-lang/rust#48655 was resolved in rust-lang/rust#72324 so this PR should eventually fix the corresponding TODO in pagecache.

This initial PR does some of the grind work of replacing bump_atomic_lsn.

NOTE: I'm not familiar with Rust's atomics, so I don't know if SeqCst is actually what you want.

For cargo test, my OS (Windows 10, WSL) couldn't hole punch the heap, and I got a lot of 60 second timeouts. Also, the tests got stuck on my end at around tree_bug_39, so I had to kill the process. It might be better on your end.

@Yam76 Yam76 changed the title Do the initial work of using fetch_max [WIP] Use fetch_max instead of bump_atomic_lsn Jan 1, 2021
@Yam76 Yam76 marked this pull request as draft January 1, 2021 01:40
@Yam76 Yam76 marked this pull request as ready for review January 1, 2021 01:45
@spacejam
Copy link
Owner

spacejam commented Jan 1, 2021

Hey @Yam76, this is explicitly avoiding fetch_max which did not stabilize until Rust 1.45. sled is currently targeting 1.39 as the minimum supported rust version (MSRV).

@spacejam spacejam closed this Jan 1, 2021
@Yam76
Copy link
Author

Yam76 commented Jan 2, 2021

D'oh, that's what I get for not reading the full README. Thank you for the response.

@spacejam
Copy link
Owner

spacejam commented Jan 2, 2021 via email

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.

2 participants