morningman commented on code in PR #68713: URL: https://github.com/apache/doris/pull/68713#discussion_r4179275791
########## regression-test/suites/external_table_p0/fluss/test_fluss_jni_heap_admission.groovy: ########## @@ -0,0 +1,94 @@ +// 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. + +// enable_jni_heap_admission has the connectors declare, on every range whose JNI reader holds much of +// BE's JVM heap, how much it will hold, and BE opens those readers only while what it admitted fits +// its budget. It is off by default. Turned on it may make a reader wait for room, and nothing else: the +// rows must come back exactly as they do with it off. +// +// The reads that declare are here: fluss primary-key buckets read whole (PK_FULL), and a union read of a +// primary-key table, whose tail is a PK_TAIL range and whose lake half the paimon connector plans. A log +// read declares nothing and is here as the case that must not change either. Each query is recorded +// with the variable on, and compared with the same query with it off; the comparison stays in the code +// because what it asserts is the agreement. +// +// Fixtures come from docker/thirdparties/docker-compose/fluss/sql/init.sql and init-lake-tail.sql, and +// are static: this suite never writes. +suite("test_fluss_jni_heap_admission", "p0,external") { + String enabled = context.config.otherConfigs.get("enableFlussTest") + if (enabled == null || !enabled.equalsIgnoreCase("true")) { + return + } + + String externalEnvIp = context.config.otherConfigs.get("externalEnvIp") + String coordinatorPort = context.config.otherConfigs.get("fluss_coordinator_port") + String minioPort = context.config.otherConfigs.get("fluss_minio_port") + String bootstrapServers = "${externalEnvIp}:${coordinatorPort}" + String catalogName = "test_fluss_jni_heap_admission" + + // Off unless a statement asks for it. + qt_default_off """show variables like 'enable_jni_heap_admission'""" + + sql """drop catalog if exists ${catalogName}""" + // required: a union read that quietly fell back to fluss alone would never plan a lake split, and + // the paimon half of the declaration would go untested. + sql """ + create catalog ${catalogName} properties ( + "type" = "fluss", + "fluss.bootstrap.servers" = "${bootstrapServers}", + "fluss.lake.paimon.s3.endpoint" = "http://${externalEnvIp}:${minioPort}";, + "fluss.lake.paimon.s3.access-key" = "minioadmin", + "fluss.lake.paimon.s3.secret-key" = "minioadmin", + "fluss.union_read.mode" = "required" + ); + """ + sql """switch ${catalogName}""" + sql """use fluss_test""" + // The C++ glue exists only for the v2 file scanner, and the session variable that picks between + // them is randomised by the fuzzy mode this pipeline runs. + sql """set enable_file_scanner_v2 = true""" + + def rowsOf = { String query -> sql(query).collect { row -> row.collect { it.toString() } } } + def sameWithAdmissionOff = { String query -> + sql """set enable_jni_heap_admission = false""" + def off = rowsOf(query) Review Comment: Fixed in 6de09492e88. The suite now reads `JvmHeapDeclaredBytes` from each query's profile -- the per-instance detail profile, fetched through the FE HTTP API as `external_table_p0/cache/condition_cache_orc` does -- and asserts that the union read declares (its `PK_TAIL` tail), that the same read with `force_jni_scanner` declares at least a megabyte (its lake half is then a paimon JNI split, which declares a 1 MB dictionary page per column per file; the tail here is about a kilobyte), and that the log read declares nothing. With the option off both union reads report 0, so these checks fail if the declarations stop reaching BE. `PK_FULL` can't be checked this way here: the environment takes a kv snapshot every ten seconds and nothing writes after init, so every snapshot already covers its bucket's whole log and the `PK_FULL` reads declare nothing (their profile shows 0). The header used to say they declared; it now says why they don't, and what a `PK_FULL` range declares stays with `FlussJniHeapEstimateTest` and `FlussSplitPlanTest`. The waiter/release path is `JniScanHeapGateTest`'s: arrival order, a release letting the next reader in, a stopped reader leaving, the longest wait. An end-to-end version would need a nonConcurrent suite that shrinks `jni_scanner_heap_budget_ratio` on the BE, which I'd keep out of p0. -- 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]
