aerospike / aerospike-client-rust

Rust client for the Aerospike database
https://www.aerospike.com/
Other
82 stars 26 forks source link

cdt_context::ctx_map_key_create not working as expected #104

Closed databasedav closed 2 years ago

databasedav commented 3 years ago

here's a repro following the example from the docs here

#[test]
fn test() {
    use aerospike::{self, operations::{maps, cdt_context}};

    let ac = aerospike::Client::new(&aerospike::ClientPolicy::default(), &"0.0.0.0:3000".to_string()).unwrap();
    let key = as_key!("test", "test", "test");
    ac.put(
        &aerospike::WritePolicy::default(),
        &key,
        &[as_bin!("m", as_map!("name" => "chuck norris"))]
    );
    ac.operate(
        &aerospike::WritePolicy::default(),
        &key,
        &[
            maps::increment_value(
                &maps::MapPolicy::default(),
                "m",
                &as_val!("jokes"),
                &as_val!(317)
            ).set_context(&[
                cdt_context::ctx_map_key_create(as_val!("stats"), maps::MapOrder::KeyOrdered),
                cdt_context::ctx_map_key_create(as_val!("accolades"), maps::MapOrder::KeyOrdered),
            ])
        ]
    );
    assert_eq!(
        "{}",
        ac.get(
            &aerospike::ReadPolicy::default(),
            &key,
            aerospike::Bins::Some(vec!["m".to_string()])
        ).unwrap().bins.get("m").unwrap().to_string()
    )
}

i did the assert_eq! as a convenience to see what's inside bin "m" and i get

thread 'tests::test' panicked at 'assertion failed: `(left == right)`
  left: `"{}"`,
 right: `"{String(\"name\"): String(\"chuck norris\")}"`' ...

when i add a .unwrap() to the ac.operate(...) above it turns out it's actually erroring with thread 'tests::test' panicked at 'called `Result::unwrap()` on an `Err` value: Error(ServerError(Unknown(26)), State { next_error: None, backtrace: InternalBacktrace { backtrace: None } })'

from here, 26 means AS_ERR_OP_NOT_APPLICABLE Operation isn't able to be applied to the current contents of the bin. which seems weird to me because this works with the python client. am i missing something?

databasedav commented 3 years ago

also semi related, the alternative pointed out at the end of the docs section isn't possible either because map::MapWriteMode is missing NoFail, which lists::ListWriteFlags has

jonas32 commented 3 years ago

Normally, the ctx_map_key method without create is also able to create bins. Can you try the method without the order parameter to be sure if the ctx_map_key_create is the problem here? https://github.com/aerospike/aerospike-client-rust/blob/master/tests/src/cdt_map.rs#L300 Here you see the way i did that in the tests of the client.

databasedav commented 3 years ago

@jonas32 using cdt_context::ctx_map_key also throws 26

databasedav commented 3 years ago

also fyi, removing the contexts works as expected

jonas32 commented 3 years ago

I suspect that the problem might be the combination of two contexts. That is also not covered in the unit tests and i never used that myself. That might be the reason why i never ran into this while implementing it. Ill have a look at that tomorrow and try to replicate it on my system.

jonas32 commented 3 years ago

Sorry for the late response. I think i found what i made wrong. https://github.com/aerospike/aerospike-client-java/blob/master/client/src/com/aerospike/client/cdt/CDT.java Before i waste time into something i got wrong: @jhecking do i see this correctly, all ctx elements are only encoded with their ID and Value and only one global flag is set for all of them (the one of the last ctx)? And range ops do not encode context flags at all? (Why?) Right now im encoding all of them with their own flag. That might be the reason it fails on multi-ctx.

jhecking commented 3 years ago

There are two different sets of flags to be considered in this case:

The CTX.mapKeyCreate function takes a MapOrder flag as a parameter, which it encodes into the ctx element's ID. (Same for CTX.listIndexCreate, which takes a ListOrder flag.)

The CDT.init function which you are referencing is only used in MapOperation.create and ListOperation.create which also take a MapOrder/ListOrder flag as parameter.

The flags typically apply to different map/list elements: With the mapKeyCreate/listIndexCreate context elements the flags apply to the map/list elements that are getting created "along the way", to create the necessary context for the actual element to which the operation applies. The flags passed to the map/list create operation apply to the element created by the operation itself. I'm a bit surprised that the flag in the latter case is applied to the ctx element at all. Maybe that's just an artifact of how the wire protocol evolved over time.

Because of this design, an interesting edge-cases is when you use a map/list create operation and apply a mapKeyCreate/listIndexCreate context to it. In this case, both the flag passed to the map/list create operation and the mapKeyCreate/listIndexCreate operation would be applied to the same ctx element! Maybe that's a rare enough edge case that it hasn't come up yet...

But in @databasedav's case, the map order flags should be applied to both ctx elements. So the problem probably lies somewhere else?

jonas32 commented 3 years ago

Hmm, i think i need to take out my debug server again. For single ctx, it works fine, with and without flags. It just breaks when a second context comes in. So that might be either a wrongly sized array indicator or something about the way the second ctx is written into the op. I'll try to dig more through the go and java client and try to find out where the difference comes in. But that will take some time. Reversing that without docs is not a fun job...

databasedav commented 3 years ago

@jonas32 what exactly is working with a single context? the example above with a single ctx_map_key_create isn't working for me either. i'm curious because for my actual use case i'm doing a list append in a single map create key context, which also doesn't work for me

jonas32 commented 3 years ago

Im also using contexts in my software. They are working fine there. But i have to say that i dont use the map_key_create but the normal map_key. In that case, it might have to do with the op itself.

databasedav commented 3 years ago

normal map_key doesn't work for me either, what is the base operation that is being done with that context that u are using? it might be a combo of that as well, since in the example the base map increment is also first creating a map and in my actual use case, the list append is also creating a list

jonas32 commented 3 years ago

My operation is a list append. That works fine. What Server version are you running that against?

databasedav commented 3 years ago

5.6.0.3

jonas32 commented 3 years ago

Could you try to run a read op like this one, just to verify that ctx is generally working in your case? I think you are probably right about that its the combination of the op you are running and the ctx. https://github.com/aerospike/aerospike-client-rust/blob/master/tests/src/cdt_map.rs#L298

databasedav commented 3 years ago

@jonas32 yup i can confirm that test works, the operation/context seems to not like doing operations on CDTs that also need to be created. i'm going to try using the cond stuff in your PR https://github.com/aerospike/aerospike-client-rust/pull/100 to try to manually do the conditional creation as a workaround for now

databasedav commented 3 years ago

@jonas32 using your PR works :) i just map put an empty list if the key doesn't exist and then do a list append with ctx_map_key

jonas32 commented 3 years ago

Great to hear that that one works :D Thats at least a workaround. Ill still look into the problem here.

databasedav commented 3 years ago

doing the opposite, removing the key if the list is emptied after a removal, panics; here's a repro

#[test]
fn test2() {
    use aerospike::{self, expressions, operations::{self, maps, cdt_context, lists}};

    let ac = aerospike::Client::new(
        &aerospike::ClientPolicy::default(), &"0.0.0.0:3000".to_string()
    ).unwrap();
    let key = aerospike::as_key!("test", "test", "test");
    ac.put(
        &aerospike::WritePolicy::default(),
        &key,
        &[aerospike::as_bin!("bin", aerospike::as_map!())]
    ).unwrap();
    ac.operate(
        &aerospike::WritePolicy::default(),
        &key,
        &[
            operations::exp::write_exp(
                "bin",
                &expressions::cond(
                    vec![
                        expressions::eq(
                            expressions::maps::get_by_key(
                                maps::MapReturnType::Count,
                                expressions::ExpType::INT,
                                expressions::string_val("key".to_string()),
                                expressions::map_bin("bin".to_string()),
                                &[]
                            ),
                            expressions::int_val(0)
                        ),
                        expressions::maps::put(
                            &maps::MapPolicy::default(),
                            expressions::string_val("key".to_string()),
                            expressions::list_val(vec![]),
                            expressions::map_bin("bin".to_string()),
                            &[]
                        ),
                        expressions::unknown(),
                    ]
                ),
                operations::exp::ExpWriteFlags::Default
            ),
            operations::lists::append(
                &operations::lists::ListPolicy::new(
                    operations::lists::ListOrderType::Ordered,
                    operations::lists::ListWriteFlags::Default
                ),
                "bin",
                &aerospike::as_val!("val")
            )
            .set_context(&[
                cdt_context::ctx_map_key(aerospike::as_val!("key")),
            ])
        ]
    ).unwrap();
    println!(
        "{:?}",
        ac.get(&aerospike::ReadPolicy::default(), &key, aerospike::Bins::All).unwrap().bins
    );
    ac.operate(
        &aerospike::WritePolicy::default(),
        &key,
        &[
            operations::lists::remove_by_value(
                "bin", &aerospike::as_val!("val"), operations::lists::ListReturnType::None
            ).set_context(&[
                operations::cdt_context::ctx_map_key(aerospike::as_val!("key"))
            ]),
            operations::exp::write_exp(
                "bin",
                &expressions::cond(vec![
                    expressions::eq(
                        expressions::lists::size(
                            expressions::map_bin("bin".to_string()),
                            &[
                                operations::cdt_context::ctx_map_key(
                                    aerospike::as_val!("key")
                                )
                            ]
                        ),
                        expressions::int_val(0)
                    ),
                    expressions::maps::remove_by_key(
                        expressions::string_val("key".to_string()),
                        expressions::map_bin("bin".to_string()),
                        &[]
                    ),
                    expressions::unknown()
                ]),
                operations::exp::ExpWriteFlags::Default
            )
        ]
    ).unwrap();
    println!(
        "{:?}",
        ac.get(&aerospike::ReadPolicy::default(), &key, aerospike::Bins::All).unwrap().bins
    );
}

gives me this thread 'tests::test2' panicked at 'index out of bounds: the len is 155 but the index is 155', .../.cargo/git/checkouts/aerospike-client-rust-34a63ba2784a38de/231a73e/src/commands/buffer.rs:1330:9

commenting out the write_exp from the second operate (the removal) shows what we expect

{"bin": HashMap({String("key"): List([String("val")])})}
{"bin": HashMap({String("key"): List([])})}

so i believe the write_exp is the culprit, assuming my cond logic is ok

this has less to do with this issue and more with the 5.6 expressions stuff so i can make a separate issue too, but wanted to post it here since it's semi-related

jonas32 commented 3 years ago

Thanks for pointing that out too. I'll have a look into the op and exp modules again, but i dont know when i will be able to do that. Comparing the Java Client to this one line by line to find the wrong part is not that fast. So again just to try to speed this up, @jhecking if there is any documentation on encoding and reference of op and exp that you can share with me, please do so. Reversing the other clients while they get changed too is not a great base to maintain this one.

databasedav commented 3 years ago

@jonas32 awesome thanks, i can help compare the clients, what should i be comparing exactly?

jhecking commented 3 years ago

@jonas32 I've reached out to some other Aerospike team members, as I don't have any additional documentation to share either. 😞