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
Secondary index traversal takes 50% longer than primary index traversal #4862
Comments
The slowdown appears to come from us checking that the secondary index key is in the range on every document: // Check whether we're out of sindex range.
ql::datum_t sindex_val; // NULL if no sindex.
if (sindex) {
// Secondary index functions are deterministic (so no need for an
// rdb_context_t) and evaluated in a pristine environment (without global
// optargs).
ql::env_t sindex_env(job.env->interruptor,
ql::return_empty_normal_batches_t::NO,
sindex->func_reql_version);
sindex_val = sindex->func->call(&sindex_env, val)->as_datum();
if (sindex->multi == sindex_multi_bool_t::MULTI
&& sindex_val.get_type() == ql::datum_t::R_ARRAY) {
boost::optional<uint64_t> tag = *ql::datum_t::extract_tag(key);
guarantee(tag);
sindex_val = sindex_val.get(*tag, ql::NOTHROW);
guarantee(sindex_val.has());
}
if (!sindex->range.contains(sindex_val)) {
return continue_bool_t::CONTINUE;
}
} I don't think we actually need to do that, unless the current secondary index key is truncated. |
Yes, it should be perfectly fine to skip that step for non-truncated keys. |
@danielmewes -- could this also be shipped in a point release? |
@coffeemug It could if necessary. I'd prefer to ship it in 2.2 otherwise. |
With the improvements: r.table("test2").between(1, 5, {index: 'survey_id'}).map(r.row.toJsonString()).count()
-> ~2.2s (i.e. only about 10% more time than for the primary key case, vs. 50% more before) |
On a table with 1 million documents and a simple secondary index on a numeric field, I'm getting the following performance:
I used the script from #4569 (comment) to generate the data.
I wouldn't expect such a drastic slow-down from traversing the secondary rather than the primary index tree.
The text was updated successfully, but these errors were encountered: