morningman commented on code in PR #68713: URL: https://github.com/apache/doris/pull/68713#discussion_r4308447325
########## be/src/util/jni_scan_heap_gate.cpp: ########## @@ -0,0 +1,256 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "util/jni_scan_heap_gate.h" + +#include <algorithm> +#include <chrono> +#include <limits> +#include <utility> + +#include "common/config.h" +#include "common/logging.h" +#include "util/jni-util.h" +#include "util/thread.h" +#include "util/time.h" + +namespace doris { + +namespace { + +// How often the gate asks the waiting readers whether their scans stopped, and looks for those that +// waited too long, although nobody gave a share back. +constexpr int64_t POLL_INTERVAL_NS = 100L * 1000 * 1000; +constexpr int64_t MB = 1024 * 1024; + +// A share of the -Xmx the JVM was started with, read from the same options the JVM was created from +// - not measured, so nothing here calls into the JVM. +int64_t jvm_heap_budget() { + const double budget = static_cast<double>(Jni::Util::get_max_jni_heap_memory_size()) * + config::jni_scanner_heap_budget_ratio; Review Comment: Fixed in d883efff472, and my reply above was wrong on one point: these would not have been the only atomic mutable configs in BE. `inverted_index_candidate_pushdown_ratio`, an mDouble read on the scan path, has been read from an atomic copy since #67180, whose author confirmed this same race there, and #67228 has since done the same for `lookup_connection_cache_block_pool_size`. The gate's two settings now follow them. Each has an atomic copy that `init()` publishes once be.conf is read and an update callback publishes on every `set_config()`, and the gate reads `config::get_jni_scanner_heap_budget_ratio()` and `config::get_jni_scanner_heap_max_wait_ms()` instead of the plain globals, relaxed since neither value publishes anything else. The tests that set the two configs by assignment (`JniScanHeapGateTest`, `ScannerContextTest`, `JniTableReaderTest`) now go through `set_config()`, which is what publishes the copies, so the wait-limit cases and the 10 MB process gate also check that the copies follow an update. What I said about the effect still holds: a decision taken with the old value or the new one is what changing the setting online means, and nothing in the account is derived from the budget. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
