Skip to content

Commit df33365

Browse files
Manuel Ungfacebook-github-bot
authored andcommitted
Fix range mtr tests
Summary: During the rebase to 8.0.28, the MEM_ROOT for quick range allocation has changed from using its own MEM_ROOT to using the main MEM_ROOT, which makes accounting trickier. While normal builds will allocate memory in large chunks and satisfy requests for memory from those chunks, asan/valgrind will convert all requests for memory into explicit allocations. This means that we allocate memory more frequently in asan/valgrind which means that we perform more frequent checks to see if we've exceeded the limit. This leads to 2 issues: - The main MEM_ROOT will already have memory allocated on it, which may already exceed the limit. The next allocation will always fail in that case. The fix is to set the limit to the already allocated amount plus the range max mem limit. There's some additional code in the error handler to ensure that the correct error message is displayed. - It's generally non-deterministic whether a query will fail or not. This is because there is still unused allocated memory on the main MEM_ROOT, and depending on how much we have left and how much the range optimizer takes, we may or may not hit an error. To fix this, I just disabled the `main.range_with_memory_limit_error` test in asan/valgrind. Squash with: D34256842 Reviewed By: hermanlee Differential Revision: D42949953 fbshipit-source-id: bfc64bf
1 parent e8a5f4a commit df33365

File tree

4 files changed

+29
-3
lines changed

4 files changed

+29
-3
lines changed

mysql-test/t/range_with_memory_limit_error.test

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
# will not be expecting errors. The error code will still be recorded in the
33
# results file though.
44

5+
# MEM_ROOT behaves differently under valgrind/asan, so more queries will hit max mem error leading to results mismatch.
6+
--source include/not_valgrind.inc
7+
--source include/not_asan.inc
8+
59
--disable_abort_on_error
610
set range_optimizer_max_mem_size = 10;
711
set range_optimizer_fail_mode = ERROR;

sql/range_optimizer/internal.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,14 @@ enum enum_range_optimizer_mem_mode { RANGE_OPT_MEM_WARN, RANGE_OPT_MEM_ERROR };
7676
class Range_optimizer_error_handler : public Internal_error_handler {
7777
public:
7878
Range_optimizer_error_handler()
79-
: m_has_errors(false), m_is_mem_error(false) {}
79+
: m_has_errors(false), m_is_mem_error(false), m_passthrough(false) {}
8080

8181
bool handle_condition(THD *thd, uint sql_errno, const char *,
8282
Sql_condition::enum_severity_level *level,
8383
const char *) override {
84+
// Do not handle any conditions if passthrough is set.
85+
if (m_passthrough) return false;
86+
8487
if (*level == Sql_condition::SL_ERROR) {
8588
m_has_errors = true;
8689
if (thd->variables.range_optimizer_fail_mode == RANGE_OPT_MEM_WARN) {
@@ -98,6 +101,18 @@ class Range_optimizer_error_handler : public Internal_error_handler {
98101
ER_THD(thd, ER_CAPACITY_EXCEEDED_IN_RANGE_OPTIMIZER));
99102
return true;
100103
}
104+
} else {
105+
// Call my_error with the correct range_optimizer_max_mem_size value.
106+
// The default error message will contain the MEMROOT limit, which may
107+
// be different from range_optimizer_max_mem_size in certain cases. To
108+
// avoid infinite recursion (since we are calling my_error in the error
109+
// handler), set m_passthrough to true.
110+
m_passthrough = true;
111+
my_error(EE_CAPACITY_EXCEEDED, MYF(0),
112+
static_cast<ulonglong>(
113+
thd->variables.range_optimizer_max_mem_size));
114+
m_passthrough = false;
115+
return true;
101116
}
102117
}
103118
return false;
@@ -108,6 +123,7 @@ class Range_optimizer_error_handler : public Internal_error_handler {
108123
private:
109124
bool m_has_errors;
110125
bool m_is_mem_error;
126+
bool m_passthrough;
111127
};
112128

113129
int index_next_different(bool is_index_scan, handler *file,

sql/range_optimizer/range_optimizer.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,12 +591,14 @@ int test_quick_select(THD *thd, MEM_ROOT *return_mem_root,
591591

592592
/* set up parameter that is passed to all functions */
593593
RANGE_OPT_PARAM param;
594+
595+
thd->push_internal_handler(&param.error_handler);
596+
auto cleanup = create_scope_guard([thd] { thd->pop_internal_handler(); });
597+
594598
if (setup_range_optimizer_param(thd, return_mem_root, temp_mem_root,
595599
keys_to_use, table, query_block, &param)) {
596600
return 0;
597601
}
598-
thd->push_internal_handler(&param.error_handler);
599-
auto cleanup = create_scope_guard([thd] { thd->pop_internal_handler(); });
600602

601603
/*
602604
Set index_merge_allowed from OPTIMIZER_SWITCH_INDEX_MERGE.

sql/sql_optimizer.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6103,7 +6103,11 @@ static ha_rows get_quick_record_count(THD *thd, JOIN_TAB *tab, ha_rows limit) {
61036103
size_t max_capacity = thd->mem_root->get_max_capacity();
61046104
bool error_for_capacity_exceeded =
61056105
thd->mem_root->get_error_for_capacity_exceeded();
6106+
// The main mem_root may already have allocated memory on it which may
6107+
// exceed range_optimizer_max_mem_size. Thus, set the max capacity to be the
6108+
// sum of the two.
61066109
thd->mem_root->set_max_capacity(
6110+
thd->mem_root->allocated_size() +
61076111
thd->variables.range_optimizer_max_mem_size);
61086112
thd->mem_root->set_error_for_capacity_exceeded(true);
61096113

0 commit comments

Comments
 (0)