airborne12 commented on code in PR #70262: URL: https://github.com/apache/doris/pull/70262#discussion_r4504887336
########## be/src/storage/index/inverted/clucene/inverted_index_writer.cpp: ########## @@ -21,6 +21,7 @@ #include "storage/index/index_file_writer.h" #include "storage/index/index_fs_directory.h" #include "storage/index/inverted/analysis/analyzer/analyzer.h" +#include "storage/index/inverted/clucene/lucene_analyzer_bridge.h" #include "storage/key_coder.h" Review Comment: Fixed in 0ca90febe5. `SDocumentsWriter::invertField()` closes the stream in a `_CLFINALLY`, so `LuceneTokenStreamBridge::close()` now drops the value's tokens and releases a batch above the high water mark (moved there from `next()`), whether the value ended or threw. `LuceneBridgeWriterTest.AFailedValueLeavesNoLargeBatchInTheBridge` fails a value after a 128 KiB batch. The analyzer itself stays with the writer until it is destroyed, since the CLucene `IndexWriter` keeps a raw pointer to it. ########## be/test/testutil/tmp_file_dirs_util.cpp: ########## @@ -0,0 +1,48 @@ +// 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 "testutil/tmp_file_dirs_util.h" + +#include <memory> +#include <string> +#include <vector> + +#include "common/check.h" +#include "common/status.h" +#include "io/fs/local_file_system.h" +#include "runtime/exec_env.h" +#include "storage/index/index_writer.h" +#include "storage/options.h" + +namespace doris::test { + +void ensure_tmp_file_dirs() { + if (ExecEnv::GetInstance()->get_tmp_file_dirs() != nullptr) { Review Comment: I'd leave this one as is: a `TmpFileDirs` whose directory a fixture deleted still works, because everything that uses the path creates it on demand. `IndexFileWriter::open()` goes through `DorisFSDirectoryFactory::getDirectory()`, which creates the index's temp directory with `create_directory()` (`std::filesystem::create_directories`, parents included), and SNII's `resolve_temp_dir()` calls `create_directories()` as well. None of the four callers of `ensure_tmp_file_dirs()` uses the path directly. The helper only has to replace a null pointer, which `CloudFileCacheWriteIndexOnlyTest` leaves behind. -- 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]
