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

Needless change in affinity function #5576

Closed
fulmicoton opened this issue Dec 9, 2024 · 2 comments · Fixed by #5580
Closed

Needless change in affinity function #5576

fulmicoton opened this issue Dec 9, 2024 · 2 comments · Fixed by #5580
Assignees
Labels
bug Something isn't working high-priority

Comments

@fulmicoton
Copy link
Contributor

fulmicoton commented Dec 9, 2024

Rust changed the endianness of the IP address internal representation.

This has an impact on its "hash", which in turn has an impact on our affinity function.

rust-lang/rust@fce1dec

As spotted and fixed by @trinity-1686a in #5564

The change is impactful for companies running quickwit with a large cache and wanting to upgrade.
We need to make sure our affinity function in 0.9 is the same as in 0.8.

@fulmicoton fulmicoton added bug Something isn't working high-priority labels Dec 9, 2024
@fulmicoton fulmicoton self-assigned this Dec 9, 2024
@fulmicoton
Copy link
Contributor Author

Actually I don't understand how rust-lang/rust@fce1dec would make a difference on most little endian (pretty much everything) arch. I assume the problem is coming from a different commit.

@trinity-1686a
Copy link
Contributor

there are two related commits:

  • rust-lang/rust@ba62034 changed how hash of ip are computed (from array/slice of u8 to a single u32)
  • rust-lang/rust@fce1dec made the hash slightly faster on big endian at the cost of being platform dependent (we only support little endian, so it has little impact on us)

fulmicoton added a commit that referenced this issue Dec 11, 2024
* Ensures the affinity function is the same as in Quickwit 0.8

Closes #5576

* Revert "fix failing test after rustc and serde update (#5564)"

This reverts commit cbf35fa.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants