You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "Aitozi (via GitHub)" <gi...@apache.org> on 2023/12/30 10:03:00 UTC

[PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Aitozi opened a new pull request, #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   
   <!-- Linking this pull request to the issue -->
   Linked issue: close #1812
   
   This PR is meant to introduce a LRU cache mode for Lookup Join, which will avoid to loading all the data to TaskManager eagerly.
   
   Currently limitation:
   
   - Only support on Primary Key table and join key should equal to the primary key
   - Only support on the fixed bucket table
   - Only support the table with `'changelog-producer' = 'lookup'`
   
   <!-- What is the purpose of the change -->
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#discussion_r1445721933


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/lookup/PrimaryKeyPartialLookupTable.java:
##########
@@ -22,35 +22,54 @@
 import org.apache.paimon.data.InternalRow;
 import org.apache.paimon.disk.IOManagerImpl;
 import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.predicate.Predicate;
+import org.apache.paimon.table.BucketMode;
 import org.apache.paimon.table.FileStoreTable;
 import org.apache.paimon.table.query.TableQuery;
+import org.apache.paimon.table.sink.FixedBucketRowKeyExtractor;
 import org.apache.paimon.table.source.DataSplit;
 import org.apache.paimon.table.source.Split;
 import org.apache.paimon.utils.Projection;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import javax.annotation.Nullable;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 
 /** Lookup table for primary key which supports to read the LSM tree directly. */
-public class PrimaryKeyPartialLookupTable implements PartialCacheLookupTable {
+public class PrimaryKeyPartialLookupTable implements LookupTable {
 
-    private static final Logger LOG = LoggerFactory.getLogger(PrimaryKeyPartialLookupTable.class);
-
-    private final FixedBucketKeyExtractor extractor;
+    private final FixedBucketRowKeyExtractor extractor;

Review Comment:
   Oh, we need a test to cover this.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "Aitozi (via GitHub)" <gi...@apache.org>.
Aitozi commented on code in PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#discussion_r1445717454


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/lookup/PrimaryKeyPartialLookupTable.java:
##########
@@ -22,35 +22,54 @@
 import org.apache.paimon.data.InternalRow;
 import org.apache.paimon.disk.IOManagerImpl;
 import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.predicate.Predicate;
+import org.apache.paimon.table.BucketMode;
 import org.apache.paimon.table.FileStoreTable;
 import org.apache.paimon.table.query.TableQuery;
+import org.apache.paimon.table.sink.FixedBucketRowKeyExtractor;
 import org.apache.paimon.table.source.DataSplit;
 import org.apache.paimon.table.source.Split;
 import org.apache.paimon.utils.Projection;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import javax.annotation.Nullable;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 
 /** Lookup table for primary key which supports to read the LSM tree directly. */
-public class PrimaryKeyPartialLookupTable implements PartialCacheLookupTable {
+public class PrimaryKeyPartialLookupTable implements LookupTable {
 
-    private static final Logger LOG = LoggerFactory.getLogger(PrimaryKeyPartialLookupTable.class);
-
-    private final FixedBucketKeyExtractor extractor;
+    private final FixedBucketRowKeyExtractor extractor;

Review Comment:
   `FixedBucketRowKeyExtractor` is project bucket key in the whole record, but in the lookup table, we should project bucket key in the primary key, can this meet the requirement ?



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#issuecomment-1884082423

   Still has one unstable case, need to take a look


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#discussion_r1440020941


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/lookup/PrimaryKeyLRULookupTable.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.flink.lookup;
+
+import org.apache.paimon.data.BinaryRow;
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.disk.IOManagerImpl;
+import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.query.TableQuery;
+import org.apache.paimon.table.source.DataSplit;
+import org.apache.paimon.table.source.Split;
+import org.apache.paimon.utils.Projection;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.function.Predicate;
+
+/** Lookup table for primary key which supports to read the LSM tree directly. */
+public class PrimaryKeyLRULookupTable implements LookupTable {
+
+    private static final Logger LOG = LoggerFactory.getLogger(PrimaryKeyLRULookupTable.class);
+
+    private final FixedBucketKeyExtractor extractor;
+
+    private final TableQuery tableQuery;
+
+    public PrimaryKeyLRULookupTable(FileStoreTable table, int[] projection, File path) {
+        this.tableQuery =
+                table.newTableQuery()
+                        .withValueProjection(Projection.of(projection).toNestedIndexes())
+                        .withIOManager(new IOManagerImpl(path.toString()));
+        this.extractor = new FixedBucketKeyExtractor(table.schema());
+    }
+
+    @Override
+    public List<InternalRow> get(InternalRow key) throws IOException {
+        extractor.setRecord(key);
+        int bucket = extractor.bucket();
+        BinaryRow partition = extractor.partition();
+
+        InternalRow kv = tableQuery.lookup(partition, bucket, key);
+        if (kv == null) {
+            return Collections.emptyList();
+        } else {
+            return Collections.singletonList(kv);
+        }
+    }
+
+    @Override
+    public void refresh(List<Split> splits) {
+        for (Split split : splits) {
+            if (!(split instanceof DataSplit)) {
+                throw new IllegalArgumentException("Unsupported split: " + split.getClass());
+            }
+            BinaryRow partition = ((DataSplit) split).partition();
+            int bucket = ((DataSplit) split).bucket();
+            List<DataFileMeta> before = ((DataSplit) split).beforeFiles();
+            List<DataFileMeta> after = ((DataSplit) split).dataFiles();
+
+            tableQuery.refreshFiles(partition, bucket, before, after);
+        }
+    }
+
+    @Override
+    public Predicate<InternalRow> recordFilter() {
+        throw new UnsupportedOperationException();

Review Comment:
   So many `UnsupportedOperationException`s, you could add an interface `FullCacheLookupTable`?



##########
paimon-core/src/main/java/org/apache/paimon/table/query/TableQueryImpl.java:
##########
@@ -0,0 +1,197 @@
+/*
+ *
+ *  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.table.query;
+
+import org.apache.paimon.CoreOptions;
+import org.apache.paimon.KeyValue;
+import org.apache.paimon.data.BinaryRow;
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.disk.IOManager;
+import org.apache.paimon.format.FileFormatDiscover;
+import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.io.KeyValueFileReaderFactory;
+import org.apache.paimon.io.cache.CacheManager;
+import org.apache.paimon.lookup.hash.HashLookupStoreFactory;
+import org.apache.paimon.mergetree.Levels;
+import org.apache.paimon.mergetree.LookupLevels;
+import org.apache.paimon.schema.KeyValueFieldsExtractor;
+import org.apache.paimon.schema.SchemaManager;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.PrimaryKeyTableUtils;
+import org.apache.paimon.types.RowType;
+import org.apache.paimon.utils.FileStorePathFactory;
+import org.apache.paimon.utils.KeyComparatorSupplier;
+import org.apache.paimon.utils.Preconditions;
+import org.apache.paimon.utils.Projection;
+
+import javax.annotation.Nullable;
+
+import java.io.IOException;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Supplier;
+
+import static org.apache.paimon.CoreOptions.LOOKUP_CACHE_MAX_MEMORY_SIZE;
+
+/** Implementation for {@link TableQuery}. */
+public class TableQueryImpl implements TableQuery {

Review Comment:
   Can you add some tests in `PrimaryKeyFileStoreTableTest`?



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkConnectorOptions.java:
##########
@@ -280,6 +280,13 @@ public class FlinkConnectorOptions {
                     .defaultValue(16)
                     .withDescription("The thread number for lookup async.");
 
+    @ExcludeFromDocumentation("Not finish yet")

Review Comment:
   `Not finish yet`? What is the next?



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkConnectorOptions.java:
##########
@@ -280,6 +280,13 @@ public class FlinkConnectorOptions {
                     .defaultValue(16)
                     .withDescription("The thread number for lookup async.");
 
+    @ExcludeFromDocumentation("Not finish yet")
+    public static final ConfigOption<LookupCacheMode> LOOKUP_CACHE_MODE =
+            ConfigOptions.key("lookup.cache.mode")

Review Comment:
   See `https://cwiki.apache.org/confluence/display/FLINK/FLIP-221%3A+Abstraction+for+lookup+source+cache+and+metric`.
   
   lookup.cache => Enum of NONE, PARTIAL and FULL
   
   We are better to be consistent with Flink.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "Aitozi (via GitHub)" <gi...@apache.org>.
Aitozi commented on code in PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#discussion_r1440024247


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/lookup/PrimaryKeyLRULookupTable.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.flink.lookup;
+
+import org.apache.paimon.data.BinaryRow;
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.disk.IOManagerImpl;
+import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.query.TableQuery;
+import org.apache.paimon.table.source.DataSplit;
+import org.apache.paimon.table.source.Split;
+import org.apache.paimon.utils.Projection;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.function.Predicate;
+
+/** Lookup table for primary key which supports to read the LSM tree directly. */
+public class PrimaryKeyLRULookupTable implements LookupTable {
+
+    private static final Logger LOG = LoggerFactory.getLogger(PrimaryKeyLRULookupTable.class);
+
+    private final FixedBucketKeyExtractor extractor;
+
+    private final TableQuery tableQuery;
+
+    public PrimaryKeyLRULookupTable(FileStoreTable table, int[] projection, File path) {
+        this.tableQuery =
+                table.newTableQuery()
+                        .withValueProjection(Projection.of(projection).toNestedIndexes())
+                        .withIOManager(new IOManagerImpl(path.toString()));
+        this.extractor = new FixedBucketKeyExtractor(table.schema());
+    }
+
+    @Override
+    public List<InternalRow> get(InternalRow key) throws IOException {
+        extractor.setRecord(key);
+        int bucket = extractor.bucket();
+        BinaryRow partition = extractor.partition();
+
+        InternalRow kv = tableQuery.lookup(partition, bucket, key);
+        if (kv == null) {
+            return Collections.emptyList();
+        } else {
+            return Collections.singletonList(kv);
+        }
+    }
+
+    @Override
+    public void refresh(List<Split> splits) {
+        for (Split split : splits) {
+            if (!(split instanceof DataSplit)) {
+                throw new IllegalArgumentException("Unsupported split: " + split.getClass());
+            }
+            BinaryRow partition = ((DataSplit) split).partition();
+            int bucket = ((DataSplit) split).bucket();
+            List<DataFileMeta> before = ((DataSplit) split).beforeFiles();
+            List<DataFileMeta> after = ((DataSplit) split).dataFiles();
+
+            tableQuery.refreshFiles(partition, bucket, before, after);
+        }
+    }
+
+    @Override
+    public Predicate<InternalRow> recordFilter() {
+        throw new UnsupportedOperationException();

Review Comment:
   yes, it looks better



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "Aitozi (via GitHub)" <gi...@apache.org>.
Aitozi commented on code in PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#discussion_r1445556308


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkConnectorOptions.java:
##########
@@ -280,6 +280,13 @@ public class FlinkConnectorOptions {
                     .defaultValue(16)
                     .withDescription("The thread number for lookup async.");
 
+    @ExcludeFromDocumentation("Not finish yet")

Review Comment:
   Fixed



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkConnectorOptions.java:
##########
@@ -280,6 +280,13 @@ public class FlinkConnectorOptions {
                     .defaultValue(16)
                     .withDescription("The thread number for lookup async.");
 
+    @ExcludeFromDocumentation("Not finish yet")
+    public static final ConfigOption<LookupCacheMode> LOOKUP_CACHE_MODE =
+            ConfigOptions.key("lookup.cache.mode")

Review Comment:
   Updated



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#discussion_r1439294641


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/lookup/PrimaryKeyLRULookupTable.java:
##########
@@ -0,0 +1,226 @@
+/*
+ * 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.flink.lookup;
+
+import org.apache.paimon.CoreOptions;
+import org.apache.paimon.KeyValue;
+import org.apache.paimon.data.BinaryRow;
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.format.FileFormatDiscover;
+import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.io.KeyValueFileReaderFactory;
+import org.apache.paimon.io.cache.CacheManager;
+import org.apache.paimon.lookup.hash.HashLookupStoreFactory;
+import org.apache.paimon.mergetree.Levels;
+import org.apache.paimon.mergetree.LookupLevels;
+import org.apache.paimon.schema.KeyValueFieldsExtractor;
+import org.apache.paimon.schema.SchemaManager;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.PrimaryKeyTableUtils;
+import org.apache.paimon.table.source.DataSplit;
+import org.apache.paimon.table.source.Split;
+import org.apache.paimon.types.RowType;
+import org.apache.paimon.utils.FileStorePathFactory;
+import org.apache.paimon.utils.KeyComparatorSupplier;
+import org.apache.paimon.utils.Preconditions;
+import org.apache.paimon.utils.Projection;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+
+import static org.apache.paimon.CoreOptions.LOOKUP_CACHE_MAX_MEMORY_SIZE;
+
+/** Lookup table for primary key which supports to read the LSM tree directly. */
+public class PrimaryKeyLRULookupTable implements LookupTable {
+
+    private static final Logger LOG = LoggerFactory.getLogger(PrimaryKeyLRULookupTable.class);
+
+    private final Map<BinaryRow, Map<Integer, LookupLevels>> tableView;
+
+    private final CoreOptions options;
+
+    private final FixedBucketKeyExtractor extractor;
+
+    private final Supplier<Comparator<InternalRow>> keyComparatorSupplier;
+
+    private final RowType keyType;
+
+    private final RowType projectedValueType;
+
+    private final KeyValueFileReaderFactory.Builder readerFactoryBuilder;
+
+    private final File path;
+
+    private final HashLookupStoreFactory hashLookupStoreFactory;
+
+    public PrimaryKeyLRULookupTable(FileStoreTable table, int[] projection, File path) {
+        this.path = path;
+        this.options = new CoreOptions(table.options());
+        this.tableView = new HashMap<>();
+        this.extractor = new FixedBucketKeyExtractor(table.schema());
+        KeyValueFieldsExtractor pkExtractor =
+                PrimaryKeyTableUtils.PrimaryKeyFieldsExtractor.EXTRACTOR;
+        this.keyType = new RowType(pkExtractor.keyFields(table.schema()));
+        CacheManager cacheManager =
+                new CacheManager(
+                        options.pageSize(),
+                        options.toConfiguration().get(LOOKUP_CACHE_MAX_MEMORY_SIZE));
+        RowType partitionType = table.schema().logicalPartitionType();
+        this.readerFactoryBuilder =
+                KeyValueFileReaderFactory.builder(
+                        table.fileIO(),
+                        new SchemaManager(table.fileIO(), table.location()),
+                        table.schema().id(),
+                        keyType,
+                        table.rowType(),
+                        FileFormatDiscover.of(options),
+                        new FileStorePathFactory(
+                                options.path(),
+                                partitionType,
+                                options.partitionDefaultName(),
+                                options.fileFormat().getFormatIdentifier()),
+                        PrimaryKeyTableUtils.PrimaryKeyFieldsExtractor.EXTRACTOR,
+                        options);
+        readerFactoryBuilder.withValueProjection(Projection.of(projection).toNestedIndexes());
+        this.projectedValueType = readerFactoryBuilder.projectedValueType();
+        this.keyComparatorSupplier = new KeyComparatorSupplier(keyType);
+        this.hashLookupStoreFactory =
+                new HashLookupStoreFactory(
+                        cacheManager,
+                        options.toConfiguration().get(CoreOptions.LOOKUP_HASH_LOAD_FACTOR));
+    }
+
+    @Override
+    public List<InternalRow> get(InternalRow key) throws IOException {
+        LookupLevels lookupLevels = getLookupLevels(key);
+        if (lookupLevels == null) {
+            return Collections.emptyList();
+        } else {
+            // lookup start from level 1.
+            KeyValue kv = lookupLevels.lookup(key, 1);
+            if (kv == null || kv.valueKind().isRetract()) {
+                return Collections.emptyList();
+            } else {
+                return Collections.singletonList(kv.value());
+            }
+        }
+    }
+
+    private LookupLevels getLookupLevels(InternalRow key) {
+        extractor.setRecord(key);
+        int bucket = extractor.bucket();
+
+        if (tableView.isEmpty()) {
+            return null;
+        }
+        // Non partitioned table
+        Map<Integer, LookupLevels> buckets = tableView.entrySet().iterator().next().getValue();
+        return buckets.get(bucket);
+    }
+
+    @Override
+    public void refresh(List<Split> splits) {
+        for (Split split : splits) {
+            if (!(split instanceof DataSplit)) {
+                throw new IllegalArgumentException("Unsupported split: " + split.getClass());
+            }
+            BinaryRow partition = ((DataSplit) split).partition();
+            int bucket = ((DataSplit) split).bucket();
+            List<DataFileMeta> before = ((DataSplit) split).beforeFiles();
+            List<DataFileMeta> after = ((DataSplit) split).dataFiles();
+
+            Map<Integer, LookupLevels> buckets =
+                    tableView.computeIfAbsent(partition, k -> new HashMap<>());
+            LookupLevels lookupLevels = buckets.get(bucket);
+
+            if (lookupLevels == null) {
+                Preconditions.checkArgument(
+                        before.isEmpty(), "The before file should be empty for the initial phase.");
+                Levels levels = new Levels(keyComparatorSupplier.get(), after, options.numLevels());
+
+                KeyValueFileReaderFactory factory = readerFactoryBuilder.build(partition, bucket);
+
+                lookupLevels =
+                        new LookupLevels(

Review Comment:
   Can you add an interface for lookup? For example:
   1. Add `FileStoreTable.newQuery(@Nullable int[] projection)`.
   2. `PrimaryKeyFileStoreTable` override `newQuery`.
   
   ```
   public interface TableQuery extends Closeable {
   
       void refreshFiles(
               BinaryRow partition,
               int bucket,
               List<DataFileMeta> beforeFiles,
               List<DataFileMeta> dataFiles);
   
       @Nullable
       InternalRow lookup(BinaryRow partition, int bucket, InternalRow key) throws IOException;
   }
   ```
   
   This can be a separate PR.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "Aitozi (via GitHub)" <gi...@apache.org>.
Aitozi commented on code in PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#discussion_r1439383947


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/lookup/PrimaryKeyLRULookupTable.java:
##########
@@ -0,0 +1,226 @@
+/*
+ * 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.flink.lookup;
+
+import org.apache.paimon.CoreOptions;
+import org.apache.paimon.KeyValue;
+import org.apache.paimon.data.BinaryRow;
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.format.FileFormatDiscover;
+import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.io.KeyValueFileReaderFactory;
+import org.apache.paimon.io.cache.CacheManager;
+import org.apache.paimon.lookup.hash.HashLookupStoreFactory;
+import org.apache.paimon.mergetree.Levels;
+import org.apache.paimon.mergetree.LookupLevels;
+import org.apache.paimon.schema.KeyValueFieldsExtractor;
+import org.apache.paimon.schema.SchemaManager;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.PrimaryKeyTableUtils;
+import org.apache.paimon.table.source.DataSplit;
+import org.apache.paimon.table.source.Split;
+import org.apache.paimon.types.RowType;
+import org.apache.paimon.utils.FileStorePathFactory;
+import org.apache.paimon.utils.KeyComparatorSupplier;
+import org.apache.paimon.utils.Preconditions;
+import org.apache.paimon.utils.Projection;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+
+import static org.apache.paimon.CoreOptions.LOOKUP_CACHE_MAX_MEMORY_SIZE;
+
+/** Lookup table for primary key which supports to read the LSM tree directly. */
+public class PrimaryKeyLRULookupTable implements LookupTable {
+
+    private static final Logger LOG = LoggerFactory.getLogger(PrimaryKeyLRULookupTable.class);
+
+    private final Map<BinaryRow, Map<Integer, LookupLevels>> tableView;
+
+    private final CoreOptions options;
+
+    private final FixedBucketKeyExtractor extractor;
+
+    private final Supplier<Comparator<InternalRow>> keyComparatorSupplier;
+
+    private final RowType keyType;
+
+    private final RowType projectedValueType;
+
+    private final KeyValueFileReaderFactory.Builder readerFactoryBuilder;
+
+    private final File path;
+
+    private final HashLookupStoreFactory hashLookupStoreFactory;
+
+    public PrimaryKeyLRULookupTable(FileStoreTable table, int[] projection, File path) {
+        this.path = path;
+        this.options = new CoreOptions(table.options());
+        this.tableView = new HashMap<>();
+        this.extractor = new FixedBucketKeyExtractor(table.schema());
+        KeyValueFieldsExtractor pkExtractor =
+                PrimaryKeyTableUtils.PrimaryKeyFieldsExtractor.EXTRACTOR;
+        this.keyType = new RowType(pkExtractor.keyFields(table.schema()));
+        CacheManager cacheManager =
+                new CacheManager(
+                        options.pageSize(),
+                        options.toConfiguration().get(LOOKUP_CACHE_MAX_MEMORY_SIZE));
+        RowType partitionType = table.schema().logicalPartitionType();
+        this.readerFactoryBuilder =
+                KeyValueFileReaderFactory.builder(
+                        table.fileIO(),
+                        new SchemaManager(table.fileIO(), table.location()),
+                        table.schema().id(),
+                        keyType,
+                        table.rowType(),
+                        FileFormatDiscover.of(options),
+                        new FileStorePathFactory(
+                                options.path(),
+                                partitionType,
+                                options.partitionDefaultName(),
+                                options.fileFormat().getFormatIdentifier()),
+                        PrimaryKeyTableUtils.PrimaryKeyFieldsExtractor.EXTRACTOR,
+                        options);
+        readerFactoryBuilder.withValueProjection(Projection.of(projection).toNestedIndexes());
+        this.projectedValueType = readerFactoryBuilder.projectedValueType();
+        this.keyComparatorSupplier = new KeyComparatorSupplier(keyType);
+        this.hashLookupStoreFactory =
+                new HashLookupStoreFactory(
+                        cacheManager,
+                        options.toConfiguration().get(CoreOptions.LOOKUP_HASH_LOAD_FACTOR));
+    }
+
+    @Override
+    public List<InternalRow> get(InternalRow key) throws IOException {
+        LookupLevels lookupLevels = getLookupLevels(key);
+        if (lookupLevels == null) {
+            return Collections.emptyList();
+        } else {
+            // lookup start from level 1.
+            KeyValue kv = lookupLevels.lookup(key, 1);
+            if (kv == null || kv.valueKind().isRetract()) {
+                return Collections.emptyList();
+            } else {
+                return Collections.singletonList(kv.value());
+            }
+        }
+    }
+
+    private LookupLevels getLookupLevels(InternalRow key) {
+        extractor.setRecord(key);
+        int bucket = extractor.bucket();
+
+        if (tableView.isEmpty()) {
+            return null;
+        }
+        // Non partitioned table
+        Map<Integer, LookupLevels> buckets = tableView.entrySet().iterator().next().getValue();
+        return buckets.get(bucket);
+    }
+
+    @Override
+    public void refresh(List<Split> splits) {
+        for (Split split : splits) {
+            if (!(split instanceof DataSplit)) {
+                throw new IllegalArgumentException("Unsupported split: " + split.getClass());
+            }
+            BinaryRow partition = ((DataSplit) split).partition();
+            int bucket = ((DataSplit) split).bucket();
+            List<DataFileMeta> before = ((DataSplit) split).beforeFiles();
+            List<DataFileMeta> after = ((DataSplit) split).dataFiles();
+
+            Map<Integer, LookupLevels> buckets =
+                    tableView.computeIfAbsent(partition, k -> new HashMap<>());
+            LookupLevels lookupLevels = buckets.get(bucket);
+
+            if (lookupLevels == null) {
+                Preconditions.checkArgument(
+                        before.isEmpty(), "The before file should be empty for the initial phase.");
+                Levels levels = new Levels(keyComparatorSupplier.get(), after, options.numLevels());
+
+                KeyValueFileReaderFactory factory = readerFactoryBuilder.build(partition, bucket);
+
+                lookupLevels =
+                        new LookupLevels(

Review Comment:
   Updated in the latest commit.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [flink] Support Flink look up based on LSM [incubator-paimon]

Posted by "Aitozi (via GitHub)" <gi...@apache.org>.
Aitozi commented on PR #2605:
URL: https://github.com/apache/incubator-paimon/pull/2605#issuecomment-1884127656

   > Still has one unstable case, need to take a look
   
   Yes, I will look into this case


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org