-
Notifications
You must be signed in to change notification settings - Fork 35
Fix for Expression Test #93
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
Conversation
jonas32
commented
Jan 25, 2021
- Adjusted the Device Size test to also support not in-memory DBs. ( Tests don't pass on fresh Master pull #91 )
- Fixed returns for HLL from the new Server version (5.4)
|
The failing test is caused by Aerospike being too fast... |
|
Why not drop the check down to 1 ms? |
|
I wanted to leave at least some time for it to not get false positives. Getting something around 10ms already looks shady to me. |
The CI env was last updated from Trusty to Xenial in Nov 2019. I just checked, and Xenial is still the default distro for Travis CI even now: https://docs.travis-ci.com/user/reference/linux/. That said, I don't see a reason not to switch to Bionic or even Focal. But what would be the benefit? (And assuming that the client still works and the tests still pass on those newer platforms, of course.) |
@kportertx Any idea why the number of records returned would have changed between 5.4 and earlier server versions? |
|
This is related to the rust client's msgpack not packing efficiently bug. The HLL library assumed efficient packing was enforced by the server's msgpack library, because it wasn't the hashed values for rust could be different from other clients. 5.4 corrected this by detecting when values were not packed efficiently and repacking them before hashing. There wasn't a way to fix this problem that would be bug compatible cross clients. Rust is the only client that I'm aware of that was inefficient for several types. Other clients had an inefficiency when packing bin/strings > 32 bytes and < 255 bytes which were being packed as str16 instead of str8. |
|
I will create a PR to update some rust stuff anyways (mainly the clippy warnings for Rust 1.49). |
|
This looks good to me. Ready to merge @jonas32? |