Closed jonas32 closed 4 years ago
Thanks so much, Jonas! First glance, this looks very good! Will take a closer look and do a proper review tomorrow.
While testing the predexp implementation, i actually found another problem.
When you create a normal filter with as_eq and a string value as second argument on a bin without secondary index, it will try to allocate much memory. The allocation seems to be done by the error return.
The test module gives:
thread 'src::query::query_single_consumer' panicked at 'Error(ServerError(IndexNotFound), State { next_error: None, backtrace: InternalBacktrace { backtrace: None } })', tests/src/query.rs:95:25
then it will crash with:
memory allocation of 269172076445696 bytes failederror: test failed, to rerun pass '--test lib'
The test always returns 269172076445696 bytes and my service returns always 38787614419.
Probably this is related to the amount of data in the set. The set from my service only holds 2 test records.
Its definitely not a full return of the set. The set only has a few KB.
I tried this with my predexp implementation version and without. This happens with both.
edit: Looks like it might be a compression problem where it tries to convert a String into i64. I found a zlib inflater in the Go client at the point where the rust one seems to fail. In Rust, this inflater seems to be missing. https://github.com/aerospike/aerospike-client-go/blob/master/multi_command.go#L120 https://github.com/aerospike/aerospike-client-rust/blob/master/src/commands/stream_command.rs#L101
To reproduce it, you need to make the as_eq filter on a String bin that does not hold an Index. It does not matter if the set exists or if it holds any data. Same result. Example:
let mut statement = Statement::new(namespace, "test_set_with_no_index", Bins::All);
statement.add_filter(as_eq!("non_exisitng_or_unindexed", "string_value"));
let rs = client.query(&qpolicy, statement).unwrap();
let mut count = 0;
for res in &*rs {
match res {
Ok(rec) => {
count += 1;
}
Err(err) => panic!(format!("{:?}", err)),
}
}
assert_eq!(count, 10);
This exact code snipped fails with
thread 'src::query::query_single_consumer' panicked at 'Error(ServerError(IndexGeneric), State { next_error: None, backtrace: InternalBacktrace { backtrace: None } })', tests/src/query.rs:96:25 test src::query::query_single_consumer ... FAILED memory allocation of 38779975527 bytes failederror: test failed, to rerun pass '--test lib'
This is what it looks like exactly in the console:
Its a pretty unstable problem. If one of the tests that run before this one fails, it wont allocate... I had the case where my server hit max index count from creating test sets. If this happens, no allocation happens... At least i could also replicate it with my own application running this exact function. It fails 100% every time.
If you put an Integer value instead of "string_value", it will not allocate. I have debugged it with valgrint and gdb. Both dont mark an issue about memleak etc. but report the high allocation value, so i guess its a parsing problem. I tried to go back the whole trace and check every buffer read step by step. If this snipped is executed, it fails. If its not executed, no problems. You can comment out the other code inside the function. As long as this 4 lines run, it will crash. (in the stream_command.rs on line 104)
if let Err(err) = conn.read_buffer(buffer::MSG_REMAINING_HEADER_SIZE as usize) {
warn!("Parse result error: {}", err);
return Err(err);
}
It does not make sense to me why it behaves like this. I compared every function that gets executed with the ones from the Golang Client. The only difference i found is the zlib stuff. Thats why i thought it might be the problem. Sadly, i was not able to get any docs about the server answer buffers from the Support. I havent tested yet if the same problem exists with the go client. Ill probably dig into the C client today and compare this one.
I will keep on investigating why it behaves like this. But ill move it to an own issue because its not related to this one so this can be merged. I think its not a critical error because it only happens under special conditions, that do not appear in production usecases.
Agree, let's merge this, and tackle the other issue separately.
This is an implementation for Predicate Filtering. Related to #70
Changes: Predicates added to query/predexp.rs Predicate Handling added to query/statement.rs Modified commands/buffer.rs to add Headers and Values for predicates Added test/example to tests/query.rs Fixed some compiler and clippy warnings in various files.
Please review and report to me if you have any ideas how to improve it. Docs still need some work.