From 40165a10835cda427d6952c267d5fae46900f5da Mon Sep 17 00:00:00 2001 From: John Esmet Date: Wed, 3 Sep 2014 14:38:04 -0400 Subject: [PATCH] FT-273 Prevent setting the memcmp magic on db handles for which a FT is already open. Improve comments. Add a test. --- ft/comparator.h | 11 +- ft/ft-ops.cc | 30 +++-- ft/ft-ops.h | 2 +- src/tests/test_memcmp_magic.cc | 209 +++++++++++++++++++++++++++++++++ src/ydb_db.cc | 6 +- 5 files changed, 241 insertions(+), 17 deletions(-) create mode 100644 src/tests/test_memcmp_magic.cc diff --git a/ft/comparator.h b/ft/comparator.h index 81a794e4afd..caf2b8b9d18 100644 --- a/ft/comparator.h +++ b/ft/comparator.h @@ -117,7 +117,10 @@ namespace toku { } public: - void create(ft_compare_func cmp, DESCRIPTOR desc, uint8_t memcmp_magic = 0) { + // This magic value is reserved to mean that the magic has not been set. + static const uint8_t MEMCMP_MAGIC_NONE = 0; + + void create(ft_compare_func cmp, DESCRIPTOR desc, uint8_t memcmp_magic = MEMCMP_MAGIC_NONE) { XCALLOC(_fake_db); init(cmp, desc, memcmp_magic); } @@ -165,8 +168,10 @@ namespace toku { int operator()(const DBT *a, const DBT *b) const { if (__builtin_expect(toku_dbt_is_infinite(a) || toku_dbt_is_infinite(b), 0)) { return toku_dbt_infinite_compare(a, b); - } else if (_memcmp_magic && dbt_has_memcmp_magic(a) - // At this point we expect b to also have the memcmp magic + } else if (_memcmp_magic != MEMCMP_MAGIC_NONE + // If `a' has the memcmp magic.. + && dbt_has_memcmp_magic(a) + // ..then we expect `b' to also have the memcmp magic && __builtin_expect(dbt_has_memcmp_magic(b), 1)) { return toku_builtin_compare_fun(nullptr, a, b); } else { diff --git a/ft/ft-ops.cc b/ft/ft-ops.cc index a48b785700e..d32b92f8a4a 100644 --- a/ft/ft-ops.cc +++ b/ft/ft-ops.cc @@ -2858,20 +2858,27 @@ toku_ft_handle_get_fanout(FT_HANDLE ft_handle, unsigned int *fanout) } } -void toku_ft_handle_set_memcmp_magic(FT_HANDLE ft_handle, uint8_t magic) { - invariant(magic != 0); - if (ft_handle->ft) { - // handle is already open, application bug if memcmp magic changes - invariant(ft_handle->ft->cmp.get_memcmp_magic() == magic); - } else { - ft_handle->options.memcmp_magic = magic; +// The memcmp magic byte may be set on a per fractal tree basis to communicate +// that if two keys begin with this byte, they may be compared with the builtin +// key comparison function. This greatly optimizes certain in-memory workloads. +int toku_ft_handle_set_memcmp_magic(FT_HANDLE ft_handle, uint8_t magic) { + if (magic == comparator::MEMCMP_MAGIC_NONE) { + return EINVAL; } + if (ft_handle->ft != nullptr) { + // if the handle is already open, then we cannot set the memcmp magic + // (because it may or may not have been set by someone else already) + return EINVAL; + } + ft_handle->options.memcmp_magic = magic; + return 0; } static int verify_builtin_comparisons_consistent(FT_HANDLE t, uint32_t flags) { - if ((flags & TOKU_DB_KEYCMP_BUILTIN) && (t->options.compare_fun != toku_builtin_compare_fun)) + if ((flags & TOKU_DB_KEYCMP_BUILTIN) && (t->options.compare_fun != toku_builtin_compare_fun)) { return EINVAL; + } return 0; } @@ -3017,6 +3024,13 @@ ft_handle_open(FT_HANDLE ft_h, const char *fname_in_env, int is_create, int only r = EINVAL; goto exit; } + + // Ensure that the memcmp magic bits are consistent, if set. + if (ft->cmp.get_memcmp_magic() != toku::comparator::MEMCMP_MAGIC_NONE && + ft_h->options.memcmp_magic != ft->cmp.get_memcmp_magic()) { + r = EINVAL; + goto exit; + } toku_ft_handle_inherit_options(ft_h, ft); if (!was_already_open) { diff --git a/ft/ft-ops.h b/ft/ft-ops.h index dae335a3cc2..c45e0c71ef5 100644 --- a/ft/ft-ops.h +++ b/ft/ft-ops.h @@ -126,7 +126,7 @@ void toku_ft_handle_set_compression_method(FT_HANDLE, enum toku_compression_meth void toku_ft_handle_get_compression_method(FT_HANDLE, enum toku_compression_method *); void toku_ft_handle_set_fanout(FT_HANDLE, unsigned int fanout); void toku_ft_handle_get_fanout(FT_HANDLE, unsigned int *fanout); -void toku_ft_handle_set_memcmp_magic(FT_HANDLE, uint8_t magic); +int toku_ft_handle_set_memcmp_magic(FT_HANDLE, uint8_t magic); void toku_ft_set_bt_compare(FT_HANDLE ft_handle, ft_compare_func cmp_func); const toku::comparator &toku_ft_get_comparator(FT_HANDLE ft_handle); diff --git a/src/tests/test_memcmp_magic.cc b/src/tests/test_memcmp_magic.cc new file mode 100644 index 00000000000..473dcaf7453 --- /dev/null +++ b/src/tests/test_memcmp_magic.cc @@ -0,0 +1,209 @@ +/* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil -*- */ +// vim: ft=cpp:expandtab:ts=8:sw=4:softtabstop=4: + +/* +COPYING CONDITIONS NOTICE: + + This program is free software; you can redistribute it and/or modify + it under the terms of version 2 of the GNU General Public License as + published by the Free Software Foundation, and provided that the + following conditions are met: + + * Redistributions of source code must retain this COPYING + CONDITIONS NOTICE, the COPYRIGHT NOTICE (below), the + DISCLAIMER (below), the UNIVERSITY PATENT NOTICE (below), the + PATENT MARKING NOTICE (below), and the PATENT RIGHTS + GRANT (below). + + * Redistributions in binary form must reproduce this COPYING + CONDITIONS NOTICE, the COPYRIGHT NOTICE (below), the + DISCLAIMER (below), the UNIVERSITY PATENT NOTICE (below), the + PATENT MARKING NOTICE (below), and the PATENT RIGHTS + GRANT (below) in the documentation and/or other materials + provided with the distribution. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + 02110-1301, USA. + +COPYRIGHT NOTICE: + + TokuFT, Tokutek Fractal Tree Indexing Library. + Copyright (C) 2014 Tokutek, Inc. + +DISCLAIMER: + + This program is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + +UNIVERSITY PATENT NOTICE: + + The technology is licensed by the Massachusetts Institute of + Technology, Rutgers State University of New Jersey, and the Research + Foundation of State University of New York at Stony Brook under + United States of America Serial No. 11/760379 and to the patents + and/or patent applications resulting from it. + +PATENT MARKING NOTICE: + + This software is covered by US Patent No. 8,185,551. + This software is covered by US Patent No. 8,489,638. + +PATENT RIGHTS GRANT: + + "THIS IMPLEMENTATION" means the copyrightable works distributed by + Tokutek as part of the Fractal Tree project. + + "PATENT CLAIMS" means the claims of patents that are owned or + licensable by Tokutek, both currently or in the future; and that in + the absence of this license would be infringed by THIS + IMPLEMENTATION or by using or running THIS IMPLEMENTATION. + + "PATENT CHALLENGE" shall mean a challenge to the validity, + patentability, enforceability and/or non-infringement of any of the + PATENT CLAIMS or otherwise opposing any of the PATENT CLAIMS. + + Tokutek hereby grants to you, for the term and geographical scope of + the PATENT CLAIMS, a non-exclusive, no-charge, royalty-free, + irrevocable (except as stated in this section) patent license to + make, have made, use, offer to sell, sell, import, transfer, and + otherwise run, modify, and propagate the contents of THIS + IMPLEMENTATION, where such license applies only to the PATENT + CLAIMS. This grant does not include claims that would be infringed + only as a consequence of further modifications of THIS + IMPLEMENTATION. If you or your agent or licensee institute or order + or agree to the institution of patent litigation against any entity + (including a cross-claim or counterclaim in a lawsuit) alleging that + THIS IMPLEMENTATION constitutes direct or contributory patent + infringement, or inducement of patent infringement, then any rights + granted to you under this License shall terminate as of the date + such litigation is filed. If you or your agent or exclusive + licensee institute or order or agree to the institution of a PATENT + CHALLENGE, then Tokutek may terminate any rights granted to you + under this License. +*/ + +#include "test.h" + +#include "util/dbt.h" + +static void test_memcmp_magic(void) { + int r; + + DB_ENV *env; + r = db_env_create(&env, 0); CKERR(r); + r = env->open(env, TOKU_TEST_FILENAME, DB_CREATE+DB_PRIVATE+DB_INIT_MPOOL+DB_INIT_TXN, 0); CKERR(r); + + DB *db; + r = db_create(&db, env, 0); CKERR(r); + + // Can't set the memcmp magic to 0 (since it's used as a sentinel for `none') + r = db->set_memcmp_magic(db, 0); CKERR2(r, EINVAL); + + // Should be ok to set it more than once, even to different things, before opening. + r = db->set_memcmp_magic(db, 1); CKERR(r); + r = db->set_memcmp_magic(db, 2); CKERR(r); + r = db->open(db, NULL, "db", NULL, DB_BTREE, DB_CREATE, 0666); CKERR(r); + + // Can't set the memcmp magic after opening. + r = db->set_memcmp_magic(db, 0); CKERR2(r, EINVAL); + r = db->set_memcmp_magic(db, 1); CKERR2(r, EINVAL); + + DB *db2; + r = db_create(&db2, env, 0); CKERR(r); + r = db2->set_memcmp_magic(db2, 3); CKERR(r); // ..we can try setting it to something different + // ..but it should fail to open + r = db2->open(db2, NULL, "db", NULL, DB_BTREE, DB_CREATE, 0666); CKERR2(r, EINVAL); + r = db2->set_memcmp_magic(db2, 2); CKERR(r); + r = db2->open(db2, NULL, "db", NULL, DB_BTREE, DB_CREATE, 0666); CKERR(r); + r = db2->close(db2, 0); + + r = db->close(db, 0); CKERR(r); + r = env->close(env, 0); CKERR(r); +} + +static int comparison_function_unused(DB *UU(db), const DBT *UU(a), const DBT *UU(b)) { + // We're testing that the memcmp magic gets used so the real + // comparison function should never get called. + invariant(false); + return 0; +} + +static int getf_key_cb(const DBT *key, const DBT *UU(val), void *extra) { + DBT *dbt = reinterpret_cast(extra); + toku_clone_dbt(dbt, *key); + return 0; +} + +static void test_memcmp_magic_sort_order(void) { + int r; + + // Verify that randomly generated integer keys are sorted in memcmp + // order when packed as little endian, even with an environment-wide + // comparison function that sorts as though keys are big-endian ints. + + DB_ENV *env; + r = db_env_create(&env, 0); CKERR(r); + r = env->set_default_bt_compare(env, comparison_function_unused); CKERR(r); + r = env->open(env, TOKU_TEST_FILENAME, DB_CREATE+DB_PRIVATE+DB_INIT_MPOOL+DB_INIT_TXN, 0); CKERR(r); + + const int magic = 49; + + DB *db; + r = db_create(&db, env, 0); CKERR(r); + r = db->set_memcmp_magic(db, magic); CKERR(r); + r = db->open(db, NULL, "db", NULL, DB_BTREE, DB_CREATE, 0666); CKERR(r); + + for (int i = 0; i < 10000; i++) { + char buf[1 + sizeof(int)]; + // Serialize key to first have the magic byte, then the little-endian key. + int k = toku_htonl(random()); + buf[0] = magic; + memcpy(&buf[1], &k, sizeof(int)); + + DBT key; + dbt_init(&key, buf, sizeof(buf)); + r = db->put(db, NULL, &key, &key, 0); CKERR(r); + } + + DB_TXN *txn; + env->txn_begin(env, NULL, &txn, 0); + DBC *dbc; + db->cursor(db, txn, &dbc, 0); + DBT prev_dbt, curr_dbt; + memset(&curr_dbt, 0, sizeof(DBT)); + memset(&prev_dbt, 0, sizeof(DBT)); + while (dbc->c_getf_next(dbc, 0, getf_key_cb, &curr_dbt)) { + invariant(curr_dbt.size == sizeof(int)); + if (prev_dbt.data != NULL) { + // Each key should be >= to the last using memcmp + int c = memcmp(prev_dbt.data, curr_dbt.data, sizeof(int)); + invariant(c <= 0); + } + toku_destroy_dbt(&prev_dbt); + prev_dbt = curr_dbt; + } + toku_destroy_dbt(&curr_dbt); + toku_destroy_dbt(&prev_dbt); + dbc->c_close(dbc); + txn->commit(txn, 0); + + r = db->close(db, 0); CKERR(r); + r = env->close(env, 0); CKERR(r); +} + +int +test_main(int argc, char *const argv[]) { + parse_args(argc, argv); + + toku_os_recursive_delete(TOKU_TEST_FILENAME); + int r = toku_os_mkdir(TOKU_TEST_FILENAME, S_IRWXU+S_IRWXG+S_IRWXO); CKERR(r); + + test_memcmp_magic(); + test_memcmp_magic_sort_order(); + + return 0; +} diff --git a/src/ydb_db.cc b/src/ydb_db.cc index f64ed4465e8..2c54a3bd4dc 100644 --- a/src/ydb_db.cc +++ b/src/ydb_db.cc @@ -729,11 +729,7 @@ toku_db_set_memcmp_magic(DB *db, uint8_t magic) { if (db_opened(db)) { return EINVAL; } - if (magic == 0) { - return EINVAL; - } - toku_ft_handle_set_memcmp_magic(db->i->ft_handle, magic); - return 0; + return toku_ft_handle_set_memcmp_magic(db->i->ft_handle, magic); } static int