jerry-024 commented on code in PR #7551: URL: https://github.com/apache/paimon/pull/7551#discussion_r3008439372
########## paimon-tantivy/paimon-tantivy-index/src/main/java/org/apache/paimon/tantivy/index/TantivyFullTextGlobalIndexWriter.java: ########## @@ -0,0 +1,213 @@ +/* + * 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. + */ + +package org.apache.paimon.tantivy.index; + +import org.apache.paimon.data.BinaryString; +import org.apache.paimon.fs.PositionOutputStream; +import org.apache.paimon.globalindex.GlobalIndexSingletonWriter; +import org.apache.paimon.globalindex.ResultEntry; +import org.apache.paimon.globalindex.io.GlobalIndexFileWriter; +import org.apache.paimon.tantivy.TantivyIndexWriter; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.Closeable; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * Full-text global index writer using Tantivy. + * + * <p>Text data is written to a local Tantivy index via JNI. On {@link #finish()}, the index + * directory is packed into a single file and written to the global index file system. + */ +public class TantivyFullTextGlobalIndexWriter implements GlobalIndexSingletonWriter, Closeable { + + private static final String FILE_NAME_PREFIX = "tantivy"; + private static final Logger LOG = + LoggerFactory.getLogger(TantivyFullTextGlobalIndexWriter.class); + + private final GlobalIndexFileWriter fileWriter; + private File tempIndexDir; + private TantivyIndexWriter writer; + private long rowId; + private boolean closed; + + public TantivyFullTextGlobalIndexWriter(GlobalIndexFileWriter fileWriter) { + this.fileWriter = fileWriter; + this.rowId = 0; + this.closed = false; + + try { + this.tempIndexDir = Files.createTempDirectory("tantivy-index-").toFile(); + this.tempIndexDir.deleteOnExit(); + this.writer = new TantivyIndexWriter(tempIndexDir.getAbsolutePath()); + } catch (IOException e) { + throw new RuntimeException("Failed to create temp index directory", e); + } + } + + @Override + public void write(Object fieldData) { + if (fieldData == null) { + rowId++; + return; + } + + String text; + if (fieldData instanceof BinaryString) { + text = fieldData.toString(); + } else if (fieldData instanceof String) { + text = (String) fieldData; + } else { + throw new IllegalArgumentException( + "Unsupported field type: " + fieldData.getClass().getName()); + } + + writer.addDocument(rowId, text); + rowId++; + } + + @Override + public List<ResultEntry> finish() { + try { + if (rowId == 0) { + return Collections.emptyList(); + } + + writer.commit(); + writer.close(); + writer = null; + + return Collections.singletonList(packIndex()); + } catch (IOException e) { + throw new RuntimeException("Failed to write Tantivy full-text global index", e); + } finally { + closed = true; + deleteTempDir(); + } Review Comment: Native resource leak in `finish()`: 1. **`rowId == 0` path**: returns early without calling `writer.close()`, then `finally` sets `closed = true` and deletes the temp dir. The subsequent `close()` method skips cleanup because `closed` is already `true`, so the native `TantivyIndexWriter` handle is never freed. 2. **Exception path**: if `writer.commit()` or `packIndex()` throws, same issue — `writer` is not closed but `closed` is set to `true`. Suggested fix — close the writer in `finally` before setting `closed`: ```java } finally { if (writer != null) { writer.close(); writer = null; } closed = true; deleteTempDir(); } ``` -- 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]
