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

Rack Awareness, Sequence Policies, More complex policies and other Fixes #140

Open
wants to merge 26 commits into
base: async
Choose a base branch
from

Conversation

CMoore-Darwinium
Copy link

@CMoore-Darwinium CMoore-Darwinium commented Jul 5, 2023

Hi @khaf , this is my attempt at a rebased pull request

It includes:

  1. Rack Awareness
  2. Sequence Policies
  3. Fixes to message encoding (particularly when dealing with larger integers)
  4. Allowing more complex policies, including using the invert flag.
  5. Bug fixes to async

@@ -27,15 +27,4 @@ pub enum Concurrency {
/// extremely large batch sizes because each node can process the request immediately. The
/// downside is extra threads will need to be created (or takedn from a thread pool).
Parallel,

/// Issue up to N commands in parallel threads. When a request completes, a new request
Copy link
Author

@CMoore-Darwinium CMoore-Darwinium Jul 5, 2023

Choose a reason for hiding this comment

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

I just took the liberty to remove this completely in order to facilitate the sequence write patch (i.e. allowing batch reads to fast retry on a replica). @jonas32 's async patch offloads the responsibility for this stuff onto the runtime, so I thought it was a good tradeoff.

@@ -26,7 +26,7 @@ rt-tokio = ["aerospike-rt/rt-tokio"]
rt-async-std = ["aerospike-rt/rt-async-std"]

[dev-dependencies]
env_logger = "0.9.3"
env_logger = "0.9"
Copy link
Author

Choose a reason for hiding this comment

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

Allow other versions of env_logger means it can keep up to date.

@@ -36,15 +36,15 @@ impl<'a> Partition<'a> {
}

pub fn new_by_key(key: &'a Key) -> Self {
let mut rdr = Cursor::new(&key.digest[0..4]);
let mut rdr = Cursor::new(&key.digest[0..2]);
Copy link
Author

@CMoore-Darwinium CMoore-Darwinium Jul 5, 2023

Choose a reason for hiding this comment

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

Made to be more similar to the C version. I can't remember why, I have a feeling that it did make a difference somehow, although it doesn't look like it.

@CMoore-Darwinium CMoore-Darwinium changed the title Caleb on top of async Rack Awareness, Sequence Policies, More complex policies and other Fixes Jul 5, 2023
@@ -89,16 +89,14 @@ impl Queue {
)
.await;

if conn.is_err() {
let Ok(Ok(conn)) = conn else {
Copy link
Author

Choose a reason for hiding this comment

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

This is a bugfix to leaked connections. In the ? return case, it would not -- the counter.

base64 = "0.11"
crossbeam-queue = "0.2"
rand = "0.7"
ripemd = "0.1"
Copy link
Author

Choose a reason for hiding this comment

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

ripemd160 was not maintained last time I checked and we were getting audit warnings

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.

1 participant