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

bug in modify index for read key response with recurse set to true #30

Closed
priyankat99 opened this issue Oct 6, 2022 · 2 comments
Closed

Comments

@priyankat99
Copy link

priyankat99 commented Oct 6, 2022

the ReadRequestResponse modify_index is currently defined as:

You can even perform blocking queries against entire subtrees of the KV store: if ?recurse is provided, the returned X-Consul-Index corresponds to the latest ModifyIndex within the prefix, and a blocking query using that ?index will wait until any key within that prefix is updated.
pub modify_index: i64,

however, i don't think the recurse option is implemented correctly. for example

suppose the original data looks something like this:
{modifyIndex: 1, key: "test"}
{modifyIndex: 2, key: "test/example"}

when i make a blocking query with ReadKeyRequest recurse set to true on the key "test", I would expect a response like this:
{modifyIndex: 2, key: "test"}
{modifyIndex: 2, key: "test/example"}

since based off the documentation, I would expect the returned x-consul-index for the key "test" to be the latest modify index within the prefix, so in this case 2, from "test/example"

however, im getting this response back even when recurse is set to true:
{modifyIndex: 1, key: "test"}
{modifyIndex: 2, key: "test/example"}

this is problematic since i want to easily find the latest modifyIndex so i can make a blocking query on the latest modifyIndex.

not sure if i am making a recursive query wrong or if this is a bug for recursive read requests on the kv store.

@kushudai
Copy link
Contributor

kushudai commented Jun 18, 2023

Hi @priyankat99 I think this may be a documentation error/ambiguity.
The consul server implementation returns all keys that match the given prefix when using recurse.
The blocking query implementation ONLY returns when any of the keys are modified after the given index but they will still return all keys with the provided prefix.

I can see the ambiguity with the statement in hindsight.
the returned X-Consul-Index corresponds to the latest ModifyIndex within the prefix, and a blocking query using that ?index will wait until any key within that prefix is updated. is worth changing to specify that all keys are returned if any key with the provided prefix has a modify index larger than the requested one.

I wrote a small/silly test to verify this:

    #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
    async fn create_and_read_key_with_recurse() {
        let consul = get_client();
        let test_id = rand::thread_rng().gen::<u32>();
        let key = format!("test/consul/read{}", test_id);
        let string_value = "This is a test";
        let res = create_or_update_key_value(&consul, &key, string_value).await;
        assert_expected_result_with_index(res, None);

        let key2 = format!("{}/with/recurse", key);
        let string_value = "This is a recurse test";
        let res2 = create_or_update_key_value(&consul, &key2, string_value).await;
        assert_expected_result_with_index(res2, None);

        let string_value = "This is a second test";
        let res = create_or_update_key_value(&consul, &key, string_value).await;
        assert_expected_result_with_index(res, None);
        let res = read_key(&consul, &key).await;
        let res2 = read_key(&consul, &key2).await;
        let initial_modify_index = res.unwrap().first().unwrap().modify_index;
        assert!(initial_modify_index > res2.unwrap().first().unwrap().modify_index);

        let new_key = key.clone();
        let read_key_req = ReadKeyRequest {
            key: &new_key,
            recurse: true,
            index: Some(initial_modify_index as u64),
            ..Default::default()
        };

        println!("Requested modify index: {}", initial_modify_index);
        let read_req = consul.read_key(read_key_req);
        let string_value = "This is a third test";
        let res = create_or_update_key_value(&consul, &key, string_value).await;
        assert_expected_result_with_index(res, None);
        println!("read_key_req: {:?}", read_req.await);
    }

This prints all keys with at least one key having a modify index larger than requested.

Requested modify index: 311
read_key_req: Ok([
    ReadKeyResponse { create_index: 309, modify_index: 312, lock_index: 0, key: "test/consul/read255272499", flags: 0, value: Some("This is a third test"), session: None },
    ReadKeyResponse { create_index: 310, modify_index: 310, lock_index: 0, key: "test/consul/read255272499/with/recurse", flags: 0, value: Some("This is a recurse test"), session: None }
])

For your specific use case, you should be able to iterate over all response keys and pick the largest modify index if I understood you correctly.
Please correct me if my understanding is inaccurate!

@badalex
Copy link
Contributor

badalex commented Jul 16, 2024

I hit this recently while using recurse=true and deleting a subkey. The problem is modify_index on the existing keys are smaller than the returned index so the next read_key call returns instantly.

I fixed it by making read_key return _index

badalex added a commit to badalex/rs-consul that referenced this issue Jul 18, 2024
when a delete occurs we need this index as the ModifyIndex of all the
ReadKeyResponses will still be the same (and lower). which causes blocking
requests to be "broken" as future read_key request immediatly returns until
either a new key, or a update occuries to a key contained in the
ReadKeyResponse Vec.

per Roblox#30
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

No branches or pull requests

3 participants