You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "dengzhhu653 (via GitHub)" <gi...@apache.org> on 2026/07/10 01:05:32 UTC
[PR] [WIP] Metastore contextual search [hive]
dengzhhu653 opened a new pull request, #6595:
URL: https://github.com/apache/hive/pull/6595
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] [WIP] Metastore contextual search [hive]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r3557015709
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/config/SearchConfig.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.hive.search.config;
+
+import java.time.Duration;
+
+import org.apache.hadoop.conf.Configuration;
+
+public record SearchConfig(Configuration configuration) {
+ public static final String REFRESH_INTERVAL_SECONDS = "metastore.search.refresh.interval.seconds";
+ public static final long REFRESH_INTERVAL_SECONDS_DEFAULT = 1L;
+
+ public static final String DEFAULT_LIMIT = "metastore.search.default.limit";
+ public static final int DEFAULT_LIMIT_DEFAULT = 10;
+
+ public static final String INIT_READY_TIMEOUT_MS = "metastore.search.init.ready.timeout.ms";
+ public static final long INIT_READY_TIMEOUT_MS_DEFAULT = 300L;
+
+ public static final String HYBRID_SEMANTIC_WEIGHT = "metastore.search.hybrid.semantic.weight";
+ public static final float HYBRID_SEMANTIC_WEIGHT_DEFAULT = 0.4f;
+
Review Comment:
Tests and other new code reference `SearchConfig.HYBRID_MATCH_WEIGHT` / `HYBRID_MATCH_WEIGHT_DEFAULT`, but these constants are not defined (only semantic weight is). This will fail compilation and prevents configuring match-vs-semantic weighting consistently.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/config/SearchConfig.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.hive.search.config;
+
+import java.time.Duration;
+
+import org.apache.hadoop.conf.Configuration;
+
+public record SearchConfig(Configuration configuration) {
+ public static final String REFRESH_INTERVAL_SECONDS = "metastore.search.refresh.interval.seconds";
+ public static final long REFRESH_INTERVAL_SECONDS_DEFAULT = 1L;
+
+ public static final String DEFAULT_LIMIT = "metastore.search.default.limit";
+ public static final int DEFAULT_LIMIT_DEFAULT = 10;
+
+ public static final String INIT_READY_TIMEOUT_MS = "metastore.search.init.ready.timeout.ms";
+ public static final long INIT_READY_TIMEOUT_MS_DEFAULT = 300L;
+
+ public static final String HYBRID_SEMANTIC_WEIGHT = "metastore.search.hybrid.semantic.weight";
+ public static final float HYBRID_SEMANTIC_WEIGHT_DEFAULT = 0.4f;
+
+ public static final String FUSION_PRIOR = "metastore.search.fusion.prior";
+ public static final float FUSION_PRIOR_DEFAULT = 0.5f;
+
+ public static final String BAYESIAN_SAMPLES = "metastore.search.bayesian.samples";
+ public static final int BAYESIAN_SAMPLES_DEFAULT = 100;
+
+ public static final String BAYESIAN_TOKENS_PER_QUERY = "metastore.search.bayesian.tokens.per.query";
+ public static final int BAYESIAN_TOKENS_PER_QUERY_DEFAULT = 5;
+
+ public static final String BAYESIAN_SEED = "metastore.search.bayesian.seed";
+ public static final long BAYESIAN_SEED_DEFAULT = 42L;
+
+ public Duration getRefreshInterval() {
+ return Duration.ofSeconds(
+ configuration.getLong(REFRESH_INTERVAL_SECONDS, REFRESH_INTERVAL_SECONDS_DEFAULT));
+ }
+
+ public int getDefaultLimit() {
+ return configuration.getInt(DEFAULT_LIMIT, DEFAULT_LIMIT_DEFAULT);
+ }
+
+ public long getInitReadyTimeoutMs() {
+ return configuration.getLong(INIT_READY_TIMEOUT_MS, INIT_READY_TIMEOUT_MS_DEFAULT);
+ }
+
+ public float getHybridSemanticWeight() {
+ float weight = configuration.getFloat(HYBRID_SEMANTIC_WEIGHT, HYBRID_SEMANTIC_WEIGHT_DEFAULT);
+ if (weight >= 1 || weight <= 0) {
+ throw new IllegalArgumentException("metastore.search.hybrid.semantic.weight " +
+ "should be configured as between 0.0 and 1.0");
+ }
+ return weight;
+ }
Review Comment:
`SearchConfig` is missing `getHybridMatchWeight()` (used by new tests and hybrid fusion logic). Also, having separate match/semantic weights risks inconsistent configuration; it’s safer if the getters treat them as complements that always sum to 1.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/HybridSearch.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.hive.search.search;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hive.search.config.SearchConfig;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.mapping.FieldSchema;
+import org.apache.hive.search.mapping.IndexMapping;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+
+/** Builds RRF queries that fuse lexical and semantic retrieval on hybrid fields. */
+public final class HybridSearch {
+ private HybridSearch() {}
+
+ public record ParsedHybridQuery(
+ String field,
+ String queryText,
+ Float semanticWeight) {}
Review Comment:
`ParsedHybridQuery` only defines `(field, queryText, semanticWeight)`, but the added tests and call sites expect both `matchWeight()` and `semanticWeight()` (and even construct it with 4 args). As-is, this will not compile and also drops the parsed `match_weight` value from requests.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/field/Field.java:
##########
@@ -0,0 +1,23 @@
+/*
+ * 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.hive.search.mapping.field;
+
+/** A single field value within a {@lin}. */
Review Comment:
The Javadoc has a broken inline tag (`{@lin}`), which will break Javadoc generation and doesn’t point readers to the intended type.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/HybridSearch.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.hive.search.search;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hive.search.config.SearchConfig;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.mapping.FieldSchema;
+import org.apache.hive.search.mapping.IndexMapping;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+
+/** Builds RRF queries that fuse lexical and semantic retrieval on hybrid fields. */
+public final class HybridSearch {
+ private HybridSearch() {}
+
+ public record ParsedHybridQuery(
+ String field,
+ String queryText,
+ Float semanticWeight) {}
+
+ public static ParsedHybridQuery parse(Object hybridBody, IndexMapping mapping)
+ throws SearchException {
+ if (hybridBody instanceof String queryText) {
+ String field = requireHybridField(mapping, null);
+ return new ParsedHybridQuery(field, queryText, null);
+ }
+ if (!(hybridBody instanceof Map<?, ?> body)) {
+ throw new SearchException("hybrid query must be a string or object");
+ }
+ @SuppressWarnings("unchecked")
+ Map<String, Object> hybridMap = (Map<String, Object>) body;
+
+ HybridWeights weights = parseHybridWeights(hybridMap);
+
Review Comment:
`parseHybridWeights(hybridMap)` can throw `IllegalArgumentException` (from `HybridWeights.validate()`), which leaks out of `parse(...)` despite the method contract declaring `throws SearchException`. This will produce unexpected unchecked failures for invalid user input instead of a `SearchException`.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/index/store/IndexBackupUtils.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hive.search.index.store;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexStateConfig;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.index.manifest.IndexManifest;
+
+public final class IndexBackupUtils {
+ private IndexBackupUtils() {}
+
+ /** Push local index state to the backup (remote) location. */
+ public static boolean syncToBackup(IndexStateClient local, IndexStateClient remote)
+ throws IOException {
+ Optional<IndexManifest> localManifest = local.readManifest();
+ if (localManifest.isEmpty()) {
+ return false;
+ }
+ Optional<IndexManifest> remoteManifest = remote.readManifest();
+ if (remoteManifest.isPresent()
+ && remoteManifest.get().lastEventId() >= localManifest.get().lastEventId()) {
+ return false;
+ }
+ applyDiff(local, remote, localManifest.get().diff(remoteManifest.orElse(null)));
+ return remote.writeManifest(localManifest.get());
+ }
+
+ /**
+ * If a previous restore left a staging manifest, finalize it when files already match,
+ * or keep it in place so {@link #restoreFromBackup} can resume.
+ */
+ public static void resolveInterruptedRestore(IndexStateClient local)
+ throws IOException {
+ Optional<IndexManifest> staging = local.readStagingManifest();
+ if (staging.isEmpty()) {
+ local.clearStagingManifest();
+ return;
+ }
Review Comment:
`resolveInterruptedRestore()` clears the staging manifest whenever `readStagingManifest()` returns empty. Since `LocalStateClient.readStagingManifest()` returns `Optional.empty()` on I/O read failure (not just when the file is absent), this can delete the staging manifest and prevent a restore from resuming after a transient read error.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/SearchReqResp.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * 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.hive.search.search;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.hive.search.exception.SearchException;
+
+public final class SearchReqResp {
+ /** Public search mode exposed on the Metastore Thrift API. */
+ public enum Mode {
+ KEYWORD,
+ SEMANTIC,
+ HYBRID
+ }
+
+ public record Request(
+ Map<String, Object> query,
+ List<String> fields,
+ int size,
+ String catalogName,
+ String databaseName) {
+ public Request {
+ query = query == null ? Map.of() : Map.copyOf(query);
+ fields = fields == null ? List.of() : List.copyOf(fields);
+ }
+
+ public static Request validated(Map<String, Object> query, List<String> fields, int size,
+ String catalogName, String databaseName) throws SearchException {
+ if (size < 0) {
+ throw new SearchException("size must be non-negative");
+ }
+ return new Request(query, fields, size, catalogName, databaseName);
+ }
+ }
+
+ public record Response(List<Map<String, Object>> hits, long total) {}
Review Comment:
`SearchReqResp.Response` doesn’t defensively copy `hits`, unlike `Request`. Since this is part of the public request/response surface, callers can mutate the provided list after construction, breaking immutability assumptions.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4320067610
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/index/Indexer.java:
##########
@@ -0,0 +1,279 @@
+/*
+ * 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.hive.search.index;
+
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ListMultimap;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.Batchable;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.inference.Embedder;
+import org.apache.hive.search.inference.EmbedderRegistry;
+import org.apache.hive.search.mapping.FieldSchema;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.mapping.TextFieldSchema;
+import org.apache.hive.search.mapping.field.Field;
+import org.apache.hive.search.mapping.field.TextField;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.index.IndexCommit;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.KeepOnlyLastCommitDeletionPolicy;
+import org.apache.lucene.index.SnapshotDeletionPolicy;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.PrefixQuery;
+import org.apache.lucene.search.Query;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class Indexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(Indexer.class);
+ private static final int EMBED_BATCH_SIZE = 256;
+
+ private final IndexManager indexManager;
+ private final EmbedderRegistry modelRegistry;
+ private final int commitFlushThreshold;
+ private SnapshotDeletionPolicy snapshotter;
+ private FlushTrackingWriter writer;
+
+ public Indexer(IndexManager index, EmbedderRegistry registry) {
+ this.indexManager = index;
+ this.modelRegistry = registry;
+ this.commitFlushThreshold =
+ new IndexOptions(index.mapping().configuration()).getCommitFlushes();
+ }
+
+ int flushesSinceCommit() {
+ return writer.flushesSinceCommit();
+ }
+
+ public void initialize() throws IOException {
+ IndexWriterConfig config =
+ new IndexWriterConfig(indexManager.mapping().analyzer())
+ .setCommitOnClose(false)
+ .setRAMBufferSizeMB(indexManager.mapping().config().getWriteBufferSizeMb());
+ SnapshotDeletionPolicy snapshotDeletionPolicy =
+ new SnapshotDeletionPolicy(new KeepOnlyLastCommitDeletionPolicy());
+ config.setIndexDeletionPolicy(snapshotDeletionPolicy);
+ this.snapshotter = snapshotDeletionPolicy;
+ this.writer = new FlushTrackingWriter(indexManager.directory(), config);
+ }
+
+ public void syncBackup() throws IOException {
+ if (!indexManager.hasBackup()) {
+ return;
+ }
+ IndexCommit snapshot = snapshotter.snapshot();
Review Comment:
The snapshot isn’t actually used by syncBackup(), so a concurrent commit can invalidate the files being copied.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/BootstrapIndexer.java:
##########
@@ -0,0 +1,291 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class BootstrapIndexer {
+ private static final Logger LOG = LoggerFactory.getLogger(BootstrapIndexer.class);
+ private static final List<TableDocument> END_OF_STREAM = List.of();
+
+ private final Configuration configuration;
+ private final IndexManager indexManager;
+ private final IndexOptions indexConfig;
+ private final Indexer indexer;
+ private final IMetaStoreClient client;
+ private final boolean shareFetchClient;
+ private Progress progress;
+
+ /** Package-private for unit tests: fetch workers reuse the injected client. */
+ BootstrapIndexer(IndexManager indexManager,
+ Indexer indexer,
+ IMetaStoreClient client,
+ boolean shareFetchClient) {
+ this.indexManager = indexManager;
+ this.configuration = indexManager.mapping().configuration();
+ this.indexConfig = new IndexOptions(configuration);
+ this.indexer = indexer;
+ this.client = client;
+ this.shareFetchClient = shareFetchClient;
+ }
+
+ void run(long notificationId) throws Exception {
+ long start = System.currentTimeMillis();
+ List<String> databases = client.getAllDatabases();
Review Comment:
This only bootstraps the default catalog. Existing tables in other catalogs will never be indexed. Do we only care about 'hive' catalog?
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/Searcher.java:
##########
@@ -0,0 +1,296 @@
+/*
+ * 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.hive.search.search;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.InferenceException;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.mapping.FieldSchema;
+import org.apache.hive.search.mapping.IndexMapping;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.mapping.TextFieldSchema;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.TableBlobCodec;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.inference.Embedder;
+import org.apache.hive.search.inference.EmbedderRegistry;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.queryparser.simple.SimpleQueryParser;
+import org.apache.lucene.search.BayesianScoreEstimator;
+import org.apache.lucene.search.BayesianScoreQuery;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.BoostQuery;
+import org.apache.lucene.search.ConstantScoreQuery;
+import org.apache.lucene.search.DisjunctionMaxQuery;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.KnnFloatVectorQuery;
+import org.apache.lucene.search.LogOddsFusionQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreDoc;
+import org.apache.lucene.search.SearcherManager;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.search.PrefixQuery;
+import org.apache.lucene.search.TopDocs;
+
+public final class Searcher implements AutoCloseable {
+ private final EmbedderRegistry embedderRegistry;
+ private final IndexSearcher indexSearcher;
+ private final SearcherManager searcherManager;
+ private final IndexMapping mapping;
+ private final SearchOptions searchConfig;
+ private final BayesianScoreEstimator.Parameters parameters;
+ private final long processedEventId;
+
+ public Searcher(SearcherManager manager,
+ IndexManager indexManager,
+ EmbedderRegistry registry,
+ SearchOptions searchConfig,
+ BayesianScoreEstimator.Parameters parameters) throws IOException {
+ this.searcherManager = manager;
+ this.processedEventId = indexManager.getProcessedEventId();
+ this.indexSearcher = manager.acquire();
+ this.mapping = indexManager.mapping();
+ this.searchConfig = searchConfig;
+ this.parameters = parameters;
+ this.embedderRegistry = Objects.requireNonNull(registry, "Embedder registry");
+ }
+
+ public TableSearchResult search(SearchQuery request)
+ throws SearchException, IOException {
+ if (StringUtils.isBlank(request.body().queryText())) {
+ throw new SearchException("query text is required");
+ }
+ int size = request.limit() > 0 ? request.limit() : searchConfig.getDefaultLimit();
+ int semanticK = searchConfig.semanticK(size);
+ Query query =
+ switch (request.body()) {
+ case MatchQuery match -> compileMatchQuery(match.resolve(mapping));
+ case SemanticQuery semantic -> compileSemanticQuery(semantic.resolve(mapping), semanticK);
+ case HybridQuery hybrid -> compileFusionQuery(hybrid.resolve(mapping), semanticK);
+ };
+ query = applyScopeFilter(query, request.catalogName(), request.databaseName());
+ TopDocs topDocs = indexSearcher.search(query, size);
+ List<TableSearchHit> hits = new ArrayList<>();
+ for (ScoreDoc scoreDoc : topDocs.scoreDocs) {
+ hits.add(readHit(scoreDoc));
+ }
+ return new TableSearchResult(hits, topDocs.totalHits.value(), processedEventId);
+ }
+
+ public TableSearchResult loadTables(List<TableName> tableNames) throws SearchException, IOException {
+ if (tableNames == null || tableNames.isEmpty()) {
+ return new TableSearchResult(List.of(), 0, processedEventId);
+ }
+ List<String> ids = new ArrayList<>(tableNames.size());
+ for (TableName table : tableNames) {
+ ids.add(MetastoreTableMapper.tableId(table));
+ }
+ BooleanQuery.Builder builder = new BooleanQuery.Builder();
+ for (String id : ids) {
+ builder.add(
+ new TermQuery(new Term("_id" + TableDocument.FILTER_SUFFIX, id)),
+ BooleanClause.Occur.SHOULD);
+ }
+ TopDocs hits = indexSearcher.search(builder.build(), ids.size());
+ List<TableSearchHit> searchHits = new ArrayList<>(hits.scoreDocs.length);
+ for (ScoreDoc scoreDoc : hits.scoreDocs) {
+ searchHits.add(readHit(scoreDoc));
+ }
+ return new TableSearchResult(searchHits, hits.totalHits.value(), processedEventId);
+ }
+
+ private Query compileFusionQuery(HybridQuery query, int semanticK)
+ throws SearchException {
+ Query matchQuery = new BayesianScoreQuery(
+ compileMatchQuery(query.toMatchQuery().resolve(mapping)),
+ parameters.alpha(), parameters.beta(), parameters.baseRate());
+ Query semanticQuery = compileSemanticQuery(query.toSemanticQuery().resolve(mapping), semanticK);
+ float semanticWeight = query.semanticWeight(searchConfig);
+ float[] weights = {1.0f - semanticWeight, semanticWeight};
+ return new LogOddsFusionQuery(List.of(matchQuery, semanticQuery),
+ searchConfig.getFusionPrior(), weights);
+ }
+
+ private Query applyScopeFilter(Query query, String catalogName, String databaseName) {
+ Query filter = compileScopeFilter(catalogName, databaseName);
+ if (filter == null) {
+ return query;
+ }
+ return new BooleanQuery.Builder()
+ .add(query, BooleanClause.Occur.MUST)
+ .add(filter, BooleanClause.Occur.FILTER)
+ .build();
+ }
+
+ private Query compileScopeFilter(String catalogName, String databaseName) {
+ boolean hasCatalog = StringUtils.isNotEmpty(catalogName);
+ boolean hasDatabase = StringUtils.isNotEmpty(databaseName);
+ if (!hasCatalog && !hasDatabase) {
+ return null;
+ }
+ if (hasCatalog && hasDatabase) {
+ return new PrefixQuery(new Term("_id" + TableDocument.FILTER_SUFFIX,
+ catalogName + "." + databaseName + "."));
+ }
+ if (hasCatalog) {
+ return new PrefixQuery(new Term("_id" + TableDocument.FILTER_SUFFIX, catalogName + "."));
+ }
+ return new TermQuery(new Term(
+ MetastoreTableMapper.FIELD_DB + TableDocument.FILTER_SUFFIX, databaseName));
+ }
+
+ private Query compileMatchQuery(MatchQuery match)
+ throws SearchException {
+ if (StringUtils.isNotEmpty(match.field())) {
+ return booleanShould(lexicalClausesForField(match.field(), match.queryText(), 1.0f),
+ "field '" + match.field() + "' is not lexically searchable");
+ }
+ BooleanQuery.Builder builder = new BooleanQuery.Builder();
+ boolean added = false;
+ for (MetastoreTableMapper.KeywordSearchField keywordField : MetastoreTableMapper.KEYWORD_SEARCH_FIELDS) {
+ for (Query clause : lexicalClausesForField(
+ keywordField.field(), match.queryText(), keywordField.boost())) {
+ builder.add(clause, BooleanClause.Occur.SHOULD);
+ added = true;
+ }
+ }
+ if (!added) {
+ throw new SearchException("No lexically searchable table fields are configured");
+ }
+ return builder.build();
+ }
+
+ private Query booleanShould(List<Query> clauses, String ifEmptyMessage) throws SearchException {
+ if (clauses.isEmpty()) {
+ throw new SearchException(ifEmptyMessage);
+ }
+ if (clauses.size() == 1) {
+ return clauses.getFirst();
+ }
+ BooleanQuery.Builder builder = new BooleanQuery.Builder();
+ for (Query clause : clauses) {
+ builder.add(clause, BooleanClause.Occur.SHOULD);
+ }
+ return builder.build();
+ }
+
+ private List<Query> lexicalClausesForField(String field, String queryText, float boost)
+ throws SearchException {
+ FieldSchema schema = mapping.fieldSchema(field);
+ if (!(schema instanceof TextFieldSchema text)) {
+ return List.of();
+ }
+ List<Query> clauses = new ArrayList<>();
+ if (text.filter()) {
+ clauses.add(maybeBoost(compileFilterKeywordQuery(field, queryText), boost));
+ if (text.lexical()) {
+ clauses.add(maybeBoost(compileFieldMatchQuery(field, queryText), boost));
+ }
+ return clauses;
+ }
+ if (text.lexical()) {
+ clauses.add(maybeBoost(compileFieldMatchQuery(field, queryText), boost));
+ }
+ return clauses;
+ }
+
+ private Query maybeBoost(Query query, float boost) {
+ return boost == 1.0f ? query : boostKeywordQuery(query, boost);
+ }
+
+ private static Query boostKeywordQuery(Query query, float boost) {
+ return new BoostQuery(new ConstantScoreQuery(query), boost);
+ }
+
+ private Query compileFilterKeywordQuery(String field, String queryText) {
+ String normalized = queryText.trim().toLowerCase(Locale.ROOT);
+ if (normalized.isEmpty()) {
+ return new BooleanQuery.Builder().build();
+ }
+ return new TermQuery(new Term(field + TableDocument.FILTER_SUFFIX, normalized));
+ }
+
+ private Query compileFieldMatchQuery(String field, String queryText)
+ throws SearchException {
+ FieldSchema schema = mapping.fieldSchema(field);
+ if (!(schema instanceof TextFieldSchema text) || !text.lexical()) {
+ throw new SearchException("field '" + field + "' is not lexically searchable");
+ }
+ return new SimpleQueryParser(mapping.analyzer(), field).parse(queryText);
+ }
+
+ private Query compileSemanticQuery(SemanticQuery semantic, int knnK) throws SearchException {
+ List<String> fields = mapping.resolveSemanticSearchFields(null);
+ FieldSchema schema = mapping.fieldSchema(fields.getFirst());
+ if (!(schema instanceof TextFieldSchema text) || !text.semantic()) {
+ throw new SearchException("index has no semantically searchable fields configured");
+ }
+ float[] embedding;
+ try {
+ embedding = embedderRegistry.get(text.semanticModel())
+ .embed(Embedder.TaskType.QUERY, semantic.queryText());
Review Comment:
pass this filter into the kNN query. Filtering after global top-K can drop all valid scoped results.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/index/store/IndexBackupUtils.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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.hive.search.index.store;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexStoreOptions;
+import org.apache.hive.search.exception.IndexIOException;
+
+public final class IndexBackupUtils {
+ private IndexBackupUtils() {}
+
+ /** Push local index state to the backup (remote) location. */
+ public static boolean syncToBackup(IndexStateClient local, IndexStateClient remote)
+ throws IOException {
+ Optional<IndexManifest> localManifest = local.readManifest();
+ if (localManifest.isEmpty()) {
+ return false;
+ }
+ Optional<IndexManifest> remoteManifest = remote.readManifest();
+ IndexManifest localM = localManifest.get();
+ if (remoteManifest.isPresent()) {
+ IndexManifest remoteM = remoteManifest.get();
+ if (remoteM.lastEventId() > localM.lastEventId()) {
+ return false;
+ }
+ if (remoteM.lastEventId() == localM.lastEventId() && localM.sameFilesAs(remoteM)) {
+ return false;
+ }
+ }
+ applyDiff(local, remote, localM.diff(remoteManifest.orElse(null)));
Review Comment:
Please stage and atomically publish the backup. A failed sync can currently corrupt the last valid backup.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreCluster.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hadoop.hive.metastore.leader.LeaderException;
+import org.apache.hadoop.hive.metastore.leader.LeaseLeaderElection;
+
+@SuppressWarnings("rawtypes, unchecked")
+public class MetastoreCluster implements AutoCloseable {
+ private static final String CLUSTER_ELECTION_METHOD = "metastore.index.election.method";
+ private static final Map<String, Pair<LeaderElection, Object>> CACHED = new ConcurrentHashMap<>();
+
+ static {
+ try {
+ CACHED.put("default", Pair.of(new LeaseLeaderElection(),
+ new TableName("__INDEX_CATALOG__", "__INDEX_DATABASE__", "__INDEX_TABLE__")));
Review Comment:
This cached election is closed after the first lifecycle and can’t be reused on reload.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/BootstrapIndexer.java:
##########
@@ -0,0 +1,291 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class BootstrapIndexer {
+ private static final Logger LOG = LoggerFactory.getLogger(BootstrapIndexer.class);
+ private static final List<TableDocument> END_OF_STREAM = List.of();
+
+ private final Configuration configuration;
+ private final IndexManager indexManager;
+ private final IndexOptions indexConfig;
+ private final Indexer indexer;
+ private final IMetaStoreClient client;
+ private final boolean shareFetchClient;
+ private Progress progress;
+
+ /** Package-private for unit tests: fetch workers reuse the injected client. */
+ BootstrapIndexer(IndexManager indexManager,
+ Indexer indexer,
+ IMetaStoreClient client,
+ boolean shareFetchClient) {
+ this.indexManager = indexManager;
+ this.configuration = indexManager.mapping().configuration();
+ this.indexConfig = new IndexOptions(configuration);
+ this.indexer = indexer;
+ this.client = client;
+ this.shareFetchClient = shareFetchClient;
+ }
+
+ void run(long notificationId) throws Exception {
+ long start = System.currentTimeMillis();
+ List<String> databases = client.getAllDatabases();
+ BatchPlan plan = planBatches(databases);
+ LOG.info("Bootstrap indexer planned {} table(s) in {} batch(es) across {} database(s)",
+ plan.plannedTableCount(), plan.batches().size(), databases.size());
+
+ BlockingQueue<List<TableDocument>> indexQueue =
+ new ArrayBlockingQueue<>(indexConfig.getBootstrapQueueDepth());
+ AtomicReference<Exception> failure = new AtomicReference<>();
+ AtomicLong indexedTables = new AtomicLong();
+ CountDownLatch fetchDone = new CountDownLatch(plan.batches().size());
+ progress = new Progress(indexedTables, plan.plannedTableCount, System.currentTimeMillis());
+ indexManager.setBootstrapIndexer(this);
+
+ Thread indexThread = startIndexConsumer(indexQueue, failure, indexedTables);
+ try (ExecutorService tableFetcher = Executors.newFixedThreadPool(indexConfig.getBootstrapFetchThreads(),
+ r -> {
+ Thread thread = new Thread(r, "Index-Bootstrap-Fetch");
+ thread.setDaemon(true);
+ return thread;
+ })) {
+ for (TableBatch batch : plan.batches()) {
+ tableFetcher.submit(() -> fetchBatch(batch, indexQueue, failure, fetchDone));
+ }
+ awaitFetchCompletion(fetchDone, failure);
+ indexQueue.put(END_OF_STREAM);
+ indexThread.join();
+ rethrowFailure(failure);
+ indexer.flush(notificationId, true);
+ indexer.syncBackup();
+ long elapsed = System.currentTimeMillis() - start;
+ LOG.info("Built index for {} tables in {} batch(es) over {}ms",
+ indexer.writer().getDocStats().numDocs, plan.batches().size(), elapsed);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IndexIOException("Bootstrap indexing interrupted", e);
+ } finally {
+ indexThread.interrupt();
+ indexThread.join(TimeUnit.SECONDS.toMillis(30));
+ }
+ }
+
+ private BatchPlan planBatches(List<String> databases) throws Exception {
+ int batchSize = indexConfig.getBootstrapBatchSize();
+ List<TableBatch> planned = new ArrayList<>();
+ int plannedTableCount = 0;
+ for (String db : databases) {
+ List<String> tableNames = client.getAllTables(db);
+ plannedTableCount += tableNames.size();
+ for (int idx = 0; idx < tableNames.size(); idx += batchSize) {
+ int end = Math.min(idx + batchSize, tableNames.size());
+ planned.add(new TableBatch(db, List.copyOf(tableNames.subList(idx, end))));
+ }
+ }
+ return new BatchPlan(planned, plannedTableCount);
+ }
+
+ private void fetchBatch(TableBatch batch,
+ BlockingQueue<List<TableDocument>> indexQueue,
+ AtomicReference<Exception> failure,
+ CountDownLatch fetchDone) {
+ try {
+ if (failure.get() != null) {
+ return;
+ }
+ if (shareFetchClient) {
+ fetchBatch(batch, indexQueue, failure, client);
+ return;
+ }
+ try (IMetaStoreClient fetchClient = RetryingMetaStoreClient.getProxy(configuration, true)) {
+ fetchBatch(batch, indexQueue, failure, fetchClient);
+ }
+ } catch (Exception e) {
+ recordFailure(failure, e);
+ } finally {
+ fetchDone.countDown();
+ }
+ }
+
+ private void fetchBatch(TableBatch batch,
+ BlockingQueue<List<TableDocument>> indexQueue,
+ AtomicReference<Exception> failure,
+ IMetaStoreClient client) throws Exception {
+ if (failure.get() != null) {
+ return;
+ }
+ List<Table> tables =
+ client.getTableObjectsByName(batch.database(), batch.tableNames());
+ List<TableDocument> documents = new ArrayList<>(tables.size());
+ for (Table table : tables) {
+ documents.add(MetastoreTableMapper.fromTable(table, indexManager.mapping()));
+ }
+ indexQueue.put(documents);
Review Comment:
If indexing fails while this queue is full, producers can block forever. Please cancel them on failure.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4320715227
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/BootstrapIndexer.java:
##########
@@ -0,0 +1,291 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class BootstrapIndexer {
+ private static final Logger LOG = LoggerFactory.getLogger(BootstrapIndexer.class);
+ private static final List<TableDocument> END_OF_STREAM = List.of();
+
+ private final Configuration configuration;
+ private final IndexManager indexManager;
+ private final IndexOptions indexConfig;
+ private final Indexer indexer;
+ private final IMetaStoreClient client;
+ private final boolean shareFetchClient;
+ private Progress progress;
+
+ /** Package-private for unit tests: fetch workers reuse the injected client. */
+ BootstrapIndexer(IndexManager indexManager,
+ Indexer indexer,
+ IMetaStoreClient client,
+ boolean shareFetchClient) {
+ this.indexManager = indexManager;
+ this.configuration = indexManager.mapping().configuration();
+ this.indexConfig = new IndexOptions(configuration);
+ this.indexer = indexer;
+ this.client = client;
+ this.shareFetchClient = shareFetchClient;
+ }
+
+ void run(long notificationId) throws Exception {
+ long start = System.currentTimeMillis();
+ List<String> databases = client.getAllDatabases();
Review Comment:
Good catch. To get a fresh search result, the catalog should support the notification, that's said, the catalog should be native to current Metastore. As of now we don't have a clear view of catalog federation, so the we only index the default catalog.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4320901975
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreCluster.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hadoop.hive.metastore.leader.LeaderException;
+import org.apache.hadoop.hive.metastore.leader.LeaseLeaderElection;
+
+@SuppressWarnings("rawtypes, unchecked")
+public class MetastoreCluster implements AutoCloseable {
+ private static final String CLUSTER_ELECTION_METHOD = "metastore.index.election.method";
+ private static final Map<String, Pair<LeaderElection, Object>> CACHED = new ConcurrentHashMap<>();
+
+ static {
+ try {
+ CACHED.put("default", Pair.of(new LeaseLeaderElection(),
+ new TableName("__INDEX_CATALOG__", "__INDEX_DATABASE__", "__INDEX_TABLE__")));
Review Comment:
Good catch, I will check if the reload is necessary or not.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4284518079
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
+ this.lastEventId = initialize();
+ this.indexManager.setNid(lastEventId);
+ }
+
+ private boolean rebuildIndex() throws Exception {
+ indexManager.resolveInterruptedRestore();
+ if (isIndexValid(indexManager.readLocalManifest().orElse(null))) {
+ return false;
+ }
+ if (isIndexValid(indexManager.readRemoteManifest().orElse(null))) {
+ if (indexManager.restoreBackup()) {
+ return false;
+ }
+ LOG.warn("Failed to restore index from remote backup; clearing partial local index");
+ indexManager.clearLocalIndex();
+ return true;
+ }
+ indexManager.clearLocalIndex();
+ return true;
+ }
+
+ private boolean isIndexValid(IndexManifest indexManifest)
Review Comment:
The design doc (§3, §3.3) defines the index mapping as a versioned contract, which is the main value of having a mapping contract. The implementation doesn’t version it, though. isIndexValid() only compares the embedder name, and Indexer.flush() only records nid, embedder and commit_time in the commit user data.
As a result, any of these changes will silently produce a mixed-shape index after an upgrade or config change, because the local or remote snapshot is still considered valid:
- adding, removing or renaming a field, or changing its lexical/filter/stored flags;
- changing an analyzer;
- changing semanticSegmentMax / semanticSegmentMaxChars (old docs keep the old segment layout);
- changing what goes into the segment text (SearchTextSegment.build), or the embedder pooling/prompt, while the model name stays the same.
Suggestion:
- Introduce an explicit MAPPING_VERSION constant that is bumped on any change in §3.3’s “mapping revision” category, and/or a fingerprint computed from the effective mapping (field names and flags, analyzer class, segment config, embedder name, pooling, dimension).
- Write it into the commit user data in Indexer.flush() and into IndexManifest.
- In isIndexValid() and waitForRemoteBackup(), treat a mismatch the same as an embedder mismatch: don’t restore, rebuild instead.
- Optionally, follow §3.3 and distinguish a stored-only change (backward compatible, no rebuild) from a searchable or analyzer change (rebuild required). A version with a “compatible since” field would cover that.
It would also be good to have a test that bumps the version and asserts that the restore path is rejected and a bootstrap runs.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5076620627
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [266 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4291025345
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/IndexMapping.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * 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.hive.search.mapping;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.config.IndexStoreOptions;
+import org.apache.hive.search.config.InferenceOptions;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.SearchTextSegment;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.analysis.core.KeywordAnalyzer;
+import org.apache.lucene.analysis.miscellaneous.PerFieldAnalyzerWrapper;
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+
+public record IndexMapping(Configuration configuration, Map<String, FieldSchema> fields) {
Review Comment:
> for the multi-segment embeddings, consider a parent/child block (one child doc per segment, a single vector field, queried with DiversifyingChildrenFloatKnnVectorQuery) instead of search_text_0..N fields. That avoids N separate HNSW graphs per index, and writer.updateDocuments(term, block) gives an atomic replace, which also fixes the delete-then-add gap in Indexer.addDocuments.
This changes the current mapping contract, I will explore it in [HIVE-30193](https://issues.apache.org/jira/browse/HIVE-30193)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4290852591
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/IndexMapping.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * 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.hive.search.mapping;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.config.IndexStoreOptions;
+import org.apache.hive.search.config.InferenceOptions;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.SearchTextSegment;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.analysis.core.KeywordAnalyzer;
+import org.apache.lucene.analysis.miscellaneous.PerFieldAnalyzerWrapper;
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+
+public record IndexMapping(Configuration configuration, Map<String, FieldSchema> fields) {
Review Comment:
This IndexMapping abstracts away from how the index is organized in the storage, and the reader(Searcher) uses this mapping to validate the input or find the index fields the current query looks for.
Another case is if we want to buy Solr/OpenSearch in the future, we don't need to care about how/what the real field for, as the mapping can be versioned, we just transform this mapping fields received to the layout the storage/persistence layer wants.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] [WIP] Metastore contextual search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5028794357
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [280 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4355826942
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
+ this.lastEventId = initialize();
+ this.indexManager.setNid(lastEventId);
+ }
+
+ private boolean rebuildIndex() throws Exception {
+ indexManager.resolveInterruptedRestore();
+ if (isIndexValid(indexManager.readLocalManifest().orElse(null))) {
+ return false;
+ }
+ if (isIndexValid(indexManager.readRemoteManifest().orElse(null))) {
+ if (indexManager.restoreBackup()) {
+ return false;
+ }
+ LOG.warn("Failed to restore index from remote backup; clearing partial local index");
+ indexManager.clearLocalIndex();
+ return true;
+ }
+ indexManager.clearLocalIndex();
+ return true;
+ }
+
+ private boolean isIndexValid(IndexManifest indexManifest)
+ throws Exception {
+ if (indexManifest == null) {
+ return false;
+ }
+ String current = indexManager.mapping().inference().embedderName();
+ String embedderName = indexManifest.embedder();
+ if (!current.equals(embedderName)) {
+ return false;
+ }
+ long notificationId = indexManifest.lastEventId();
+ return canCatchUp(notificationId);
+ }
+
+ private boolean canCatchUp(long notificationId) throws Exception {
+ if (notificationId < 0) {
+ return false;
+ }
+ try {
+ NotificationEventRequest request = new NotificationEventRequest(notificationId);
+ request.setMaxEvents(10);
+ client.getNextNotification(request, false, null);
+ return true;
+ } catch (IllegalStateException ignored) {
+ LOG.debug("Current index lags too behind, will rebuild");
+ }
+ return false;
+ }
+
+ private long initialize() throws Exception {
+ boolean rebuild = rebuildIndex();
+ if (rebuild && !cluster.isLeader()) {
+ long notificationId = waitForRemoteBackup();
+ indexer.initialize();
+ if (notificationId >= 0) {
+ return notificationId;
+ }
+ return rebuildLeaderIndex();
+ }
+ indexer.initialize();
+ if (rebuild) {
+ return rebuildLeaderIndex();
+ }
+ return indexManager.readLocalManifest().get().lastEventId();
+ }
+
+ private long rebuildLeaderIndex() throws Exception {
+ indexManager.clearRemoteIndex();
+ long notificationId = client.getCurrentNotificationEventId().getEventId();
+ new BootstrapIndexer(
+ indexManager,
+ indexer,
+ client,
+ shareBootstrapFetchClient).run(notificationId);
+ return notificationId;
+ }
+
+ public void start() throws Exception {
+ handler.start(lastEventId);
Review Comment:
Good catch, we should avoid this race condition
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-6357767242
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [307 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4321519537
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreCluster.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hadoop.hive.metastore.leader.LeaderException;
+import org.apache.hadoop.hive.metastore.leader.LeaseLeaderElection;
+
+@SuppressWarnings("rawtypes, unchecked")
+public class MetastoreCluster implements AutoCloseable {
+ private static final String CLUSTER_ELECTION_METHOD = "metastore.index.election.method";
+ private static final Map<String, Pair<LeaderElection, Object>> CACHED = new ConcurrentHashMap<>();
+
+ static {
+ try {
+ CACHED.put("default", Pair.of(new LeaseLeaderElection(),
+ new TableName("__INDEX_CATALOG__", "__INDEX_DATABASE__", "__INDEX_TABLE__")));
Review Comment:
Cleared the reload in SearchProvider
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreCluster.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hadoop.hive.metastore.leader.LeaderException;
+import org.apache.hadoop.hive.metastore.leader.LeaseLeaderElection;
+
+@SuppressWarnings("rawtypes, unchecked")
+public class MetastoreCluster implements AutoCloseable {
+ private static final String CLUSTER_ELECTION_METHOD = "metastore.index.election.method";
+ private static final Map<String, Pair<LeaderElection, Object>> CACHED = new ConcurrentHashMap<>();
+
+ static {
+ try {
+ CACHED.put("default", Pair.of(new LeaseLeaderElection(),
+ new TableName("__INDEX_CATALOG__", "__INDEX_DATABASE__", "__INDEX_TABLE__")));
Review Comment:
Removed the reload in SearchProvider
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4321513257
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/index/Indexer.java:
##########
@@ -0,0 +1,279 @@
+/*
+ * 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.hive.search.index;
+
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ListMultimap;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.Batchable;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.inference.Embedder;
+import org.apache.hive.search.inference.EmbedderRegistry;
+import org.apache.hive.search.mapping.FieldSchema;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.mapping.TextFieldSchema;
+import org.apache.hive.search.mapping.field.Field;
+import org.apache.hive.search.mapping.field.TextField;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.index.IndexCommit;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.KeepOnlyLastCommitDeletionPolicy;
+import org.apache.lucene.index.SnapshotDeletionPolicy;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.PrefixQuery;
+import org.apache.lucene.search.Query;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class Indexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(Indexer.class);
+ private static final int EMBED_BATCH_SIZE = 256;
+
+ private final IndexManager indexManager;
+ private final EmbedderRegistry modelRegistry;
+ private final int commitFlushThreshold;
+ private SnapshotDeletionPolicy snapshotter;
+ private FlushTrackingWriter writer;
+
+ public Indexer(IndexManager index, EmbedderRegistry registry) {
+ this.indexManager = index;
+ this.modelRegistry = registry;
+ this.commitFlushThreshold =
+ new IndexOptions(index.mapping().configuration()).getCommitFlushes();
+ }
+
+ int flushesSinceCommit() {
+ return writer.flushesSinceCommit();
+ }
+
+ public void initialize() throws IOException {
+ IndexWriterConfig config =
+ new IndexWriterConfig(indexManager.mapping().analyzer())
+ .setCommitOnClose(false)
+ .setRAMBufferSizeMB(indexManager.mapping().config().getWriteBufferSizeMb());
+ SnapshotDeletionPolicy snapshotDeletionPolicy =
+ new SnapshotDeletionPolicy(new KeepOnlyLastCommitDeletionPolicy());
+ config.setIndexDeletionPolicy(snapshotDeletionPolicy);
+ this.snapshotter = snapshotDeletionPolicy;
+ this.writer = new FlushTrackingWriter(indexManager.directory(), config);
+ }
+
+ public void syncBackup() throws IOException {
+ if (!indexManager.hasBackup()) {
+ return;
+ }
+ IndexCommit snapshot = snapshotter.snapshot();
Review Comment:
done
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-6343189342
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [321 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] [WIP] Metastore contextual search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5008935287
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [329 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] [WIP] Metastore contextual search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5013658314
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [292 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] [WIP] Metastore contextual search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5020789676
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [278 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5138090414
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [250 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4290852591
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/IndexMapping.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * 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.hive.search.mapping;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.config.IndexStoreOptions;
+import org.apache.hive.search.config.InferenceOptions;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.SearchTextSegment;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.analysis.core.KeywordAnalyzer;
+import org.apache.lucene.analysis.miscellaneous.PerFieldAnalyzerWrapper;
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+
+public record IndexMapping(Configuration configuration, Map<String, FieldSchema> fields) {
Review Comment:
This IndexMapping abstracts away from how the index is organized in the storage, and the reader(Searcher) uses this mapping to validate the input or find the fields the current query searches for.
Another case is if we want to buy Solr/OpenSearch in the future, we don't need to care about how/what the real field for, as the mapping can be versioned, we just transform this mapping fields received to the layout the storage/persistence layer wants.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-6186338350
Hi @saihemanth-cloudera @okumin @zhangbutao this PR has been pending for months, can I request a review to get it done?
We can document this feature as experimented, and will be evolving on the way, and the implementation will be loosely coupled from the current codes/modules, so we can deactivate/remove this feature easily if found none of use.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5105073112
@jintongjiang could you take a look if have secs please?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4290094889
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
+ this.lastEventId = initialize();
+ this.indexManager.setNid(lastEventId);
+ }
+
+ private boolean rebuildIndex() throws Exception {
+ indexManager.resolveInterruptedRestore();
+ if (isIndexValid(indexManager.readLocalManifest().orElse(null))) {
+ return false;
+ }
+ if (isIndexValid(indexManager.readRemoteManifest().orElse(null))) {
+ if (indexManager.restoreBackup()) {
+ return false;
+ }
+ LOG.warn("Failed to restore index from remote backup; clearing partial local index");
+ indexManager.clearLocalIndex();
+ return true;
+ }
+ indexManager.clearLocalIndex();
+ return true;
+ }
+
+ private boolean isIndexValid(IndexManifest indexManifest)
Review Comment:
I get the idea, it means the index schema version(index mapping), it might be changed, we need to rebuild(bootstrap) the index if the old index is not compatible with the new schema. Filed HIVE-30193 as the follow-up task.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4284488177
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/IndexMapping.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * 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.hive.search.mapping;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.config.IndexStoreOptions;
+import org.apache.hive.search.config.InferenceOptions;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.SearchTextSegment;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.analysis.core.KeywordAnalyzer;
+import org.apache.lucene.analysis.miscellaneous.PerFieldAnalyzerWrapper;
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+
+public record IndexMapping(Configuration configuration, Map<String, FieldSchema> fields) {
Review Comment:
The mapping package (FieldSchema, TextFieldSchema, BinaryFieldSchema, IndexMapping, Field/TextField/IdField/BinaryField, TableDocument) is essentially a hand-rolled object→document mapper, but we only have a single document type with ~10 fixed fields. Lucene’s own field classes already are the mapping layer, so this adds indirection without buying flexibility.
Two alternatives worth considering:
**Option A**: Hibernate Search (standalone POJO mapper). This is what the design doc already points to for Phase 3. hibernate-search-mapper-pojo-standalone doesn’t require Hibernate ORM, so it works with our Thrift Table (or a thin TableIndexView record). It is Apache-2.0 licensed as of 8.x, so it is fine for an ASF project. It would give us:
- declarative mapping (@FullTextField, @KeywordField, @VectorField, @DocumentId) in place of the schema/field class hierarchy;
- analyzer configuration, mass indexing (MassIndexer), which could replace BootstrapIndexer’s fetch/queue/writer plumbing, and atomic document updates out of the box;
- the embedded-Lucene ↔ Elasticsearch/OpenSearch backend switch that §2.8 / Phase 3 describes, without us maintaining SearchBackend ourselves.
Things to check before committing to it: which Lucene version its Lucene backend pins (we’re on 10.5 and use BayesianScoreQuery / LogOddsFusionQuery; the hybrid fusion would likely need the native Lucene query extension); and the extra dependency footprint in the HMS classpath.
**Option B**: collapse the framework. If we’d rather avoid the dependency, a single TableDocumentBuilder that turns a Table (plus embeddings) into a Lucene Document directly, plus a TableIndexFields constants class, would replace most of the package:
````
final class TableDocumentBuilder {
static Document build(Table t, List<float[]> segmentVectors) {
Document d = new Document();
String id = TableIndexFields.id(t);
d.add(new StringField(ID, id, Store.YES));
d.add(new StringField(DB_FILTER, t.getDbName(), Store.NO));
d.add(new TextField(TABLE, t.getTableName().toLowerCase(Locale.ROOT), Store.NO));
d.add(new TextField(COLUMNS, columnNames(t), Store.YES));
d.add(new TextField(COMMENT, comment(t), Store.YES));
...
return d;
}
}
````
Analyzers then stay in one PerFieldAnalyzerWrapper, and the “which fields are lexical/semantic/filter” knowledge lives in one place instead of being split between MetastoreIndexSchema, TextFieldSchema flags, TableDocument.toDocumentField and IndexMapping.resolve*.
Related: for the multi-segment embeddings, consider a parent/child block (one child doc per segment, a single vector field, queried with DiversifyingChildrenFloatKnnVectorQuery) instead of search_text_0..N fields. That avoids N separate HNSW graphs per index, and writer.updateDocuments(term, block) gives an atomic replace, which also fixes the delete-then-add gap in Indexer.addDocuments.
Small things in the current code: TableDocument.toDocuments() mutates a member Document, so a second call duplicates fields; it returns List<Document>, but blocks are never used; and IndexMapping.resolveSemanticSearchFields has a dead statement (SearchTextSegment.isSegmentField(fieldName);, result discarded).
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4289865241
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
+ this.lastEventId = initialize();
+ this.indexManager.setNid(lastEventId);
+ }
+
+ private boolean rebuildIndex() throws Exception {
+ indexManager.resolveInterruptedRestore();
+ if (isIndexValid(indexManager.readLocalManifest().orElse(null))) {
+ return false;
+ }
+ if (isIndexValid(indexManager.readRemoteManifest().orElse(null))) {
+ if (indexManager.restoreBackup()) {
+ return false;
+ }
+ LOG.warn("Failed to restore index from remote backup; clearing partial local index");
+ indexManager.clearLocalIndex();
+ return true;
+ }
+ indexManager.clearLocalIndex();
+ return true;
+ }
+
+ private boolean isIndexValid(IndexManifest indexManifest)
Review Comment:
The `notificationId` plays the versioned contract, it implies the latest notification has been flushed into local index. If the local index can catch up with the remote notifications starting from this `notificationId`, then we can say the local index is valid, and apply the notifications incrementally other than rebuilding the whole index.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4320761558
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/BootstrapIndexer.java:
##########
@@ -0,0 +1,291 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class BootstrapIndexer {
+ private static final Logger LOG = LoggerFactory.getLogger(BootstrapIndexer.class);
+ private static final List<TableDocument> END_OF_STREAM = List.of();
+
+ private final Configuration configuration;
+ private final IndexManager indexManager;
+ private final IndexOptions indexConfig;
+ private final Indexer indexer;
+ private final IMetaStoreClient client;
+ private final boolean shareFetchClient;
+ private Progress progress;
+
+ /** Package-private for unit tests: fetch workers reuse the injected client. */
+ BootstrapIndexer(IndexManager indexManager,
+ Indexer indexer,
+ IMetaStoreClient client,
+ boolean shareFetchClient) {
+ this.indexManager = indexManager;
+ this.configuration = indexManager.mapping().configuration();
+ this.indexConfig = new IndexOptions(configuration);
+ this.indexer = indexer;
+ this.client = client;
+ this.shareFetchClient = shareFetchClient;
+ }
+
+ void run(long notificationId) throws Exception {
+ long start = System.currentTimeMillis();
+ List<String> databases = client.getAllDatabases();
+ BatchPlan plan = planBatches(databases);
+ LOG.info("Bootstrap indexer planned {} table(s) in {} batch(es) across {} database(s)",
+ plan.plannedTableCount(), plan.batches().size(), databases.size());
+
+ BlockingQueue<List<TableDocument>> indexQueue =
+ new ArrayBlockingQueue<>(indexConfig.getBootstrapQueueDepth());
+ AtomicReference<Exception> failure = new AtomicReference<>();
+ AtomicLong indexedTables = new AtomicLong();
+ CountDownLatch fetchDone = new CountDownLatch(plan.batches().size());
+ progress = new Progress(indexedTables, plan.plannedTableCount, System.currentTimeMillis());
+ indexManager.setBootstrapIndexer(this);
+
+ Thread indexThread = startIndexConsumer(indexQueue, failure, indexedTables);
+ try (ExecutorService tableFetcher = Executors.newFixedThreadPool(indexConfig.getBootstrapFetchThreads(),
+ r -> {
+ Thread thread = new Thread(r, "Index-Bootstrap-Fetch");
+ thread.setDaemon(true);
+ return thread;
+ })) {
+ for (TableBatch batch : plan.batches()) {
+ tableFetcher.submit(() -> fetchBatch(batch, indexQueue, failure, fetchDone));
+ }
+ awaitFetchCompletion(fetchDone, failure);
+ indexQueue.put(END_OF_STREAM);
+ indexThread.join();
+ rethrowFailure(failure);
+ indexer.flush(notificationId, true);
+ indexer.syncBackup();
+ long elapsed = System.currentTimeMillis() - start;
+ LOG.info("Built index for {} tables in {} batch(es) over {}ms",
+ indexer.writer().getDocStats().numDocs, plan.batches().size(), elapsed);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IndexIOException("Bootstrap indexing interrupted", e);
+ } finally {
+ indexThread.interrupt();
+ indexThread.join(TimeUnit.SECONDS.toMillis(30));
+ }
+ }
+
+ private BatchPlan planBatches(List<String> databases) throws Exception {
+ int batchSize = indexConfig.getBootstrapBatchSize();
+ List<TableBatch> planned = new ArrayList<>();
+ int plannedTableCount = 0;
+ for (String db : databases) {
+ List<String> tableNames = client.getAllTables(db);
+ plannedTableCount += tableNames.size();
+ for (int idx = 0; idx < tableNames.size(); idx += batchSize) {
+ int end = Math.min(idx + batchSize, tableNames.size());
+ planned.add(new TableBatch(db, List.copyOf(tableNames.subList(idx, end))));
+ }
+ }
+ return new BatchPlan(planned, plannedTableCount);
+ }
+
+ private void fetchBatch(TableBatch batch,
+ BlockingQueue<List<TableDocument>> indexQueue,
+ AtomicReference<Exception> failure,
+ CountDownLatch fetchDone) {
+ try {
+ if (failure.get() != null) {
+ return;
+ }
+ if (shareFetchClient) {
+ fetchBatch(batch, indexQueue, failure, client);
+ return;
+ }
+ try (IMetaStoreClient fetchClient = RetryingMetaStoreClient.getProxy(configuration, true)) {
+ fetchBatch(batch, indexQueue, failure, fetchClient);
+ }
+ } catch (Exception e) {
+ recordFailure(failure, e);
+ } finally {
+ fetchDone.countDown();
+ }
+ }
+
+ private void fetchBatch(TableBatch batch,
+ BlockingQueue<List<TableDocument>> indexQueue,
+ AtomicReference<Exception> failure,
+ IMetaStoreClient client) throws Exception {
+ if (failure.get() != null) {
+ return;
+ }
+ List<Table> tables =
+ client.getTableObjectsByName(batch.database(), batch.tableNames());
+ List<TableDocument> documents = new ArrayList<>(tables.size());
+ for (Table table : tables) {
+ documents.add(MetastoreTableMapper.fromTable(table, indexManager.mapping()));
+ }
+ indexQueue.put(documents);
Review Comment:
The consumer will record the indexing failure, the `awaitFetchCompletion(fetchDone, failure)` sees the exception, and stops the bootstrap indexing early, this will trigger a shutdown message to the `tableFetcher`.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4355825103
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/Searcher.java:
##########
@@ -0,0 +1,293 @@
+/*
+ * 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.hive.search.search;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.InferenceException;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.mapping.FieldSchema;
+import org.apache.hive.search.mapping.IndexMapping;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.mapping.TextFieldSchema;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.TableBlobCodec;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.inference.Embedder;
+import org.apache.hive.search.inference.EmbedderRegistry;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.queryparser.simple.SimpleQueryParser;
+import org.apache.lucene.search.BayesianScoreEstimator;
+import org.apache.lucene.search.BayesianScoreQuery;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.BoostQuery;
+import org.apache.lucene.search.ConstantScoreQuery;
+import org.apache.lucene.search.DisjunctionMaxQuery;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.KnnFloatVectorQuery;
+import org.apache.lucene.search.LogOddsFusionQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreDoc;
+import org.apache.lucene.search.SearcherManager;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.search.PrefixQuery;
+import org.apache.lucene.search.TopDocs;
+
+public final class Searcher implements AutoCloseable {
+ private final EmbedderRegistry embedderRegistry;
+ private final IndexSearcher indexSearcher;
+ private final SearcherManager searcherManager;
+ private final IndexMapping mapping;
+ private final SearchOptions searchConfig;
+ private final BayesianScoreEstimator.Parameters parameters;
+ private final long processedEventId;
+
+ public Searcher(SearcherManager manager,
+ IndexManager indexManager,
+ EmbedderRegistry registry,
+ SearchOptions searchConfig,
+ BayesianScoreEstimator.Parameters parameters) throws IOException {
+ this.searcherManager = manager;
+ this.processedEventId = indexManager.getProcessedEventId();
+ this.indexSearcher = manager.acquire();
+ this.mapping = indexManager.mapping();
+ this.searchConfig = searchConfig;
+ this.parameters = parameters;
+ this.embedderRegistry = Objects.requireNonNull(registry, "Embedder registry");
+ }
+
+ public TableSearchResult search(SearchQuery request)
+ throws SearchException, IOException {
+ if (StringUtils.isBlank(request.body().queryText())) {
+ throw new SearchException("query text is required");
+ }
+ Query filter = compileScopeFilter(request.catalogName(), request.databaseName());
+ int size = request.limit() > 0 ? request.limit() : searchConfig.getDefaultLimit();
+ Query query =
+ switch (request.body()) {
+ case MatchQuery match -> {
+ Query matchQuery = compileMatchQuery(match.resolve(mapping));
+ if (filter == null) {
+ yield matchQuery;
+ }
+ yield new BooleanQuery.Builder()
+ .add(matchQuery, BooleanClause.Occur.MUST)
+ .add(filter, BooleanClause.Occur.FILTER)
+ .build();
+ }
+ case SemanticQuery semantic -> compileSemanticQuery(semantic.resolve(mapping), size, filter);
+ case HybridQuery hybrid -> compileFusionQuery(hybrid.resolve(mapping), size, filter);
+ };
+ TopDocs topDocs = indexSearcher.search(query, size);
+ List<TableSearchHit> hits = new ArrayList<>();
+ for (ScoreDoc scoreDoc : topDocs.scoreDocs) {
+ hits.add(readHit(scoreDoc));
+ }
+ return new TableSearchResult(hits, topDocs.totalHits.value(), processedEventId);
+ }
+
+ public TableSearchResult loadTables(List<TableName> tableNames) throws SearchException, IOException {
+ if (tableNames == null || tableNames.isEmpty()) {
+ return new TableSearchResult(List.of(), 0, processedEventId);
+ }
+ List<String> ids = new ArrayList<>(tableNames.size());
+ for (TableName table : tableNames) {
+ ids.add(MetastoreTableMapper.tableId(table));
+ }
+ BooleanQuery.Builder builder = new BooleanQuery.Builder();
+ for (String id : ids) {
+ builder.add(
+ new TermQuery(new Term("_id" + TableDocument.FILTER_SUFFIX, id)),
+ BooleanClause.Occur.SHOULD);
+ }
+ TopDocs hits = indexSearcher.search(builder.build(), ids.size());
+ List<TableSearchHit> searchHits = new ArrayList<>(hits.scoreDocs.length);
+ for (ScoreDoc scoreDoc : hits.scoreDocs) {
+ searchHits.add(readHit(scoreDoc));
+ }
+ return new TableSearchResult(searchHits, hits.totalHits.value(), processedEventId);
+ }
+
+ private Query compileFusionQuery(HybridQuery query, int semanticK, Query filter)
+ throws SearchException {
+ Query matchQuery = new BayesianScoreQuery(
Review Comment:
Done
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5448284617
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [297 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5060306436
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [270 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-6194122760
ok, it would take some time, PR is just massive :)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-6205609751
> > Clients already connect to Metastore over Thrift
>
> do we support REST client here as well or is that a follow-up?
We can implement the REST later if found useful, as of now the Thrift(will be added in follow-up) is the first choice, so the client can talk/auth with the server in the old way.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4320850817
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/index/store/IndexBackupUtils.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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.hive.search.index.store;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexStoreOptions;
+import org.apache.hive.search.exception.IndexIOException;
+
+public final class IndexBackupUtils {
+ private IndexBackupUtils() {}
+
+ /** Push local index state to the backup (remote) location. */
+ public static boolean syncToBackup(IndexStateClient local, IndexStateClient remote)
+ throws IOException {
+ Optional<IndexManifest> localManifest = local.readManifest();
+ if (localManifest.isEmpty()) {
+ return false;
+ }
+ Optional<IndexManifest> remoteManifest = remote.readManifest();
+ IndexManifest localM = localManifest.get();
+ if (remoteManifest.isPresent()) {
+ IndexManifest remoteM = remoteManifest.get();
+ if (remoteM.lastEventId() > localM.lastEventId()) {
+ return false;
+ }
+ if (remoteM.lastEventId() == localM.lastEventId() && localM.sameFilesAs(remoteM)) {
+ return false;
+ }
+ }
+ applyDiff(local, remote, localM.diff(remoteManifest.orElse(null)));
Review Comment:
We don't need to stage the backup, as we will write a remote manifest at the end of backup to audit the event.
If the last valid backup is corrupt, that doesn't matter, the leader Metastore will sync the backup in 10s,
https://github.com/dengzhhu653/hive/blob/a51964ba08e8d3ff4eae5e85a8ceccdb32c87d0a/standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java#L277
For other Metastore instances, it will check the remote manifest during restoring the index, so a corrupted index is prevented until the remote index is ready again.
Adding the stage and atomical publication would increase the complexity, it benefits less if we can restore the remote backup in a short period.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4284488177
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/IndexMapping.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * 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.hive.search.mapping;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.config.IndexStoreOptions;
+import org.apache.hive.search.config.InferenceOptions;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.SearchTextSegment;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.analysis.core.KeywordAnalyzer;
+import org.apache.lucene.analysis.miscellaneous.PerFieldAnalyzerWrapper;
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+
+public record IndexMapping(Configuration configuration, Map<String, FieldSchema> fields) {
Review Comment:
The mapping package (FieldSchema, TextFieldSchema, BinaryFieldSchema, IndexMapping, Field/TextField/IdField/BinaryField, TableDocument) is essentially a hand-rolled object→document mapper, but we only have a single document type with ~10 fixed fields. Lucene’s own field classes already are the mapping layer, so this adds indirection without buying flexibility.
Two alternatives worth considering:
Option A: Hibernate Search (standalone POJO mapper). This is what the design doc already points to for Phase 3. hibernate-search-mapper-pojo-standalone doesn’t require Hibernate ORM, so it works with our Thrift Table (or a thin TableIndexView record). It is Apache-2.0 licensed as of 8.x, so it is fine for an ASF project. It would give us:
- declarative mapping (@FullTextField, @KeywordField, @VectorField, @DocumentId) in place of the schema/field class hierarchy;
- analyzer configuration, mass indexing (MassIndexer), which could replace BootstrapIndexer’s fetch/queue/writer plumbing, and atomic document updates out of the box;
- the embedded-Lucene ↔ Elasticsearch/OpenSearch backend switch that §2.8 / Phase 3 describes, without us maintaining SearchBackend ourselves.
Things to check before committing to it: which Lucene version its Lucene backend pins (we’re on 10.5 and use BayesianScoreQuery / LogOddsFusionQuery; the hybrid fusion would likely need the native Lucene query extension); and the extra dependency footprint in the HMS classpath.
Option B: collapse the framework. If we’d rather avoid the dependency, a single TableDocumentBuilder that turns a Table (plus embeddings) into a Lucene Document directly, plus a TableIndexFields constants class, would replace most of the package:
````
final class TableDocumentBuilder {
static Document build(Table t, List<float[]> segmentVectors) {
Document d = new Document();
String id = TableIndexFields.id(t);
d.add(new StringField(ID, id, Store.YES));
d.add(new StringField(DB_FILTER, t.getDbName(), Store.NO));
d.add(new TextField(TABLE, t.getTableName().toLowerCase(Locale.ROOT), Store.NO));
d.add(new TextField(COLUMNS, columnNames(t), Store.YES));
d.add(new TextField(COMMENT, comment(t), Store.YES));
...
return d;
}
}
````
Analyzers then stay in one PerFieldAnalyzerWrapper, and the “which fields are lexical/semantic/filter” knowledge lives in one place instead of being split between MetastoreIndexSchema, TextFieldSchema flags, TableDocument.toDocumentField and IndexMapping.resolve*.
Related: for the multi-segment embeddings, consider a parent/child block (one child doc per segment, a single vector field, queried with DiversifyingChildrenFloatKnnVectorQuery) instead of search_text_0..N fields. That avoids N separate HNSW graphs per index, and writer.updateDocuments(term, block) gives an atomic replace, which also fixes the delete-then-add gap in Indexer.addDocuments.
Small things in the current code: TableDocument.toDocuments() mutates a member Document, so a second call duplicates fields; it returns List<Document>, but blocks are never used; and IndexMapping.resolveSemanticSearchFields has a dead statement (SearchTextSegment.isSegmentField(fieldName);, result discarded).
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4320715227
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/BootstrapIndexer.java:
##########
@@ -0,0 +1,291 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class BootstrapIndexer {
+ private static final Logger LOG = LoggerFactory.getLogger(BootstrapIndexer.class);
+ private static final List<TableDocument> END_OF_STREAM = List.of();
+
+ private final Configuration configuration;
+ private final IndexManager indexManager;
+ private final IndexOptions indexConfig;
+ private final Indexer indexer;
+ private final IMetaStoreClient client;
+ private final boolean shareFetchClient;
+ private Progress progress;
+
+ /** Package-private for unit tests: fetch workers reuse the injected client. */
+ BootstrapIndexer(IndexManager indexManager,
+ Indexer indexer,
+ IMetaStoreClient client,
+ boolean shareFetchClient) {
+ this.indexManager = indexManager;
+ this.configuration = indexManager.mapping().configuration();
+ this.indexConfig = new IndexOptions(configuration);
+ this.indexer = indexer;
+ this.client = client;
+ this.shareFetchClient = shareFetchClient;
+ }
+
+ void run(long notificationId) throws Exception {
+ long start = System.currentTimeMillis();
+ List<String> databases = client.getAllDatabases();
Review Comment:
Good catch. To get a fresh search result, the catalog should support the notification, that's said, the catalog should be native to current Metastore. As of now we don't have a clear view of catalog federation, so we only index the default catalog.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] [WIP] Metastore contextual search [hive]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r3607273917
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/server/SearchProvider.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.hive.search.server;
+
+import java.io.IOException;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.exception.IndexNotReadyException;
+import org.apache.hive.search.exception.InitializeException;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.search.LuceneSearchBackend;
+import org.apache.hive.search.search.SearchBackend;
+import org.apache.hive.search.search.SearchQuery;
+import org.apache.hive.search.search.TableSearchResult;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** Single server-side provider that owns the lifecycle of the {@link SearchBackend}. */
+public final class SearchProvider implements AutoCloseable {
+ private static final Logger LOG = LoggerFactory.getLogger(SearchProvider.class);
+ private static final AtomicReference<SearchProvider> INSTANCE = new AtomicReference<>();
+
+ private final Configuration configuration;
+ private final SearchBackend backend;
+
+ private SearchProvider(Configuration configuration, SearchBackend backend) {
+ this.configuration = configuration;
+ this.backend = backend;
+ }
+
+ /** Creates and initializes a provider with the supplied backend implementation. */
+ private static SearchProvider create(Configuration configuration, SearchBackend backend)
+ throws InitializeException, IOException {
+ Objects.requireNonNull(configuration, "configuration");
+ Objects.requireNonNull(backend, "backend");
+ backend.initialize(configuration);
+ LOG.info("Installed search backend {}", backend.getClass().getSimpleName());
+ return new SearchProvider(configuration, backend);
+ }
+
+ /** Installs the process-wide provider, replacing any previous instance. */
+ public static SearchProvider install(Configuration configuration)
+ throws InitializeException, IOException {
+ return install(configuration, new LuceneSearchBackend());
+ }
+
+ /** Installs the process-wide provider using the supplied backend implementation. */
+ public static SearchProvider install(Configuration configuration, SearchBackend backend)
+ throws InitializeException, IOException {
+ SearchProvider provider;
+ if ((provider = INSTANCE.get()) != null) {
+ return provider;
+ }
+ synchronized (SearchProvider.class) {
+ if ((provider = INSTANCE.get()) == null) {
+ provider = create(configuration, backend);
+ INSTANCE.getAndSet(provider);
+ }
+ }
+ return provider;
+ }
Review Comment:
`install(Configuration, SearchBackend)` claims to replace any previous instance (and tests expect that), but it currently short-circuits and returns the existing provider when `INSTANCE` is already set. This prevents swapping backends/configurations and also leaves the previous backend unclosed, so `installReplacesPreviousProvider()` will fail.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/server/SearchProvider.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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.hive.search.server;
+
+import java.io.IOException;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.exception.IndexNotReadyException;
+import org.apache.hive.search.exception.InitializeException;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.search.LuceneSearchBackend;
+import org.apache.hive.search.search.SearchBackend;
+import org.apache.hive.search.search.SearchQuery;
+import org.apache.hive.search.search.TableSearchResult;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** Single server-side provider that owns the lifecycle of the {@link SearchBackend}. */
+public final class SearchProvider implements AutoCloseable {
+ private static final Logger LOG = LoggerFactory.getLogger(SearchProvider.class);
+ private static final AtomicReference<SearchProvider> INSTANCE = new AtomicReference<>();
+
+ private final Configuration configuration;
+ private final SearchBackend backend;
+
+ private SearchProvider(Configuration configuration, SearchBackend backend) {
+ this.configuration = configuration;
+ this.backend = backend;
+ }
+
+ /** Creates and initializes a provider with the supplied backend implementation. */
+ private static SearchProvider create(Configuration configuration, SearchBackend backend)
+ throws InitializeException, IOException {
+ Objects.requireNonNull(configuration, "configuration");
+ Objects.requireNonNull(backend, "backend");
+ backend.initialize(configuration);
+ LOG.info("Installed search backend {}", backend.getClass().getSimpleName());
+ return new SearchProvider(configuration, backend);
+ }
+
+ /** Installs the process-wide provider, replacing any previous instance. */
+ public static SearchProvider install(Configuration configuration)
+ throws InitializeException, IOException {
+ return install(configuration, new LuceneSearchBackend());
+ }
+
+ /** Installs the process-wide provider using the supplied backend implementation. */
+ public static SearchProvider install(Configuration configuration, SearchBackend backend)
+ throws InitializeException, IOException {
+ SearchProvider provider;
+ if ((provider = INSTANCE.get()) != null) {
+ return provider;
+ }
+ synchronized (SearchProvider.class) {
+ if ((provider = INSTANCE.get()) == null) {
+ provider = create(configuration, backend);
+ INSTANCE.getAndSet(provider);
+ }
+ }
+ return provider;
+ }
+
+ /** Returns the installed provider. */
+ public static SearchProvider get() {
+ SearchProvider provider = INSTANCE.get();
+ if (provider == null) {
+ throw new IllegalStateException("SearchServerProvider is not installed");
+ }
Review Comment:
The exception message in `get()` references `SearchServerProvider`, but the class is `SearchProvider`. This makes debugging/conf logs confusing.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] [WIP] Metastore contextual search [hive]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r3556956765
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/index/store/IndexBackupUtils.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hive.search.index.store;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexStateConfig;
+import org.apache.hive.search.exception.IndexIOException;
+import org.apache.hive.search.index.manifest.IndexManifest;
+
+public final class IndexBackupUtils {
+ private IndexBackupUtils() {}
+
+ /** Push local index state to the backup (remote) location. */
+ public static boolean syncToBackup(IndexStateClient local, IndexStateClient remote)
+ throws IOException {
+ Optional<IndexManifest> localManifest = local.readManifest();
+ if (localManifest.isEmpty()) {
+ return false;
+ }
+ Optional<IndexManifest> remoteManifest = remote.readManifest();
+ if (remoteManifest.isPresent()
+ && remoteManifest.get().lastEventId() >= localManifest.get().lastEventId()) {
+ return false;
+ }
+ applyDiff(local, remote, localManifest.get().diff(remoteManifest.orElse(null)));
+ return remote.writeManifest(localManifest.get());
+ }
+
+ /**
+ * If a previous restore left a staging manifest, finalize it when files already match,
+ * or keep it in place so {@link #restoreFromBackup} can resume.
+ */
+ public static void resolveInterruptedRestore(IndexStateClient local)
+ throws IOException {
+ Optional<IndexManifest> staging = local.readStagingManifest();
+ if (staging.isEmpty()) {
+ local.clearStagingManifest();
+ return;
+ }
Review Comment:
resolveInterruptedRestore() clears the staging manifest when readStagingManifest() returns Optional.empty(). LocalStateClient.readStagingManifest() also returns empty on read/parsing failure, so this can delete a real staging manifest and prevent restoreFromBackup() from resuming after a partial/corrupted staging read.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/HybridSearch.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.hive.search.search;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hive.search.config.SearchConfig;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.mapping.FieldSchema;
+import org.apache.hive.search.mapping.IndexMapping;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+
+/** Builds RRF queries that fuse lexical and semantic retrieval on hybrid fields. */
+public final class HybridSearch {
+ private HybridSearch() {}
+
+ public record ParsedHybridQuery(
+ String field,
+ String queryText,
+ Float semanticWeight) {}
+
+ public static ParsedHybridQuery parse(Object hybridBody, IndexMapping mapping)
+ throws SearchException {
+ if (hybridBody instanceof String queryText) {
+ String field = requireHybridField(mapping, null);
+ return new ParsedHybridQuery(field, queryText, null);
+ }
+ if (!(hybridBody instanceof Map<?, ?> body)) {
+ throw new SearchException("hybrid query must be a string or object");
+ }
+ @SuppressWarnings("unchecked")
+ Map<String, Object> hybridMap = (Map<String, Object>) body;
+
+ HybridWeights weights = parseHybridWeights(hybridMap);
Review Comment:
HybridSearch.parseHybridWeights() can throw IllegalArgumentException for invalid weight values, but HybridSearch.parse() only declares SearchException. This leaks an unchecked exception out through SearchInternal.compileHybridQuery(), bypassing the expected SearchException path and likely surfacing as an unexpected runtime failure instead of a user-facing validation error.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/lucene/search/package-info.java:
##########
@@ -0,0 +1,21 @@
+/*
+ * 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.
+ */
+
+/**
+ * Classes in this package are copied from Lucene codebase.
+ */
+package org.apache.lucene.search;
Review Comment:
Introducing new classes under the upstream Lucene package (org.apache.lucene.search) creates a split-package with the Lucene dependency and increases the risk of classpath/module-path conflicts or future Lucene name collisions. Prefer placing copied/adapted code under an org.apache.hive.* namespace (or shading/relocating) and referencing it from there.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/field/Field.java:
##########
@@ -0,0 +1,23 @@
+/*
+ * 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.hive.search.mapping.field;
+
+/** A single field value within a {@lin}. */
Review Comment:
Javadoc link tag is misspelled ("{@lin}") which will render incorrectly and may trigger Javadoc warnings.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5098946163
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [239 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4290094889
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
+ this.lastEventId = initialize();
+ this.indexManager.setNid(lastEventId);
+ }
+
+ private boolean rebuildIndex() throws Exception {
+ indexManager.resolveInterruptedRestore();
+ if (isIndexValid(indexManager.readLocalManifest().orElse(null))) {
+ return false;
+ }
+ if (isIndexValid(indexManager.readRemoteManifest().orElse(null))) {
+ if (indexManager.restoreBackup()) {
+ return false;
+ }
+ LOG.warn("Failed to restore index from remote backup; clearing partial local index");
+ indexManager.clearLocalIndex();
+ return true;
+ }
+ indexManager.clearLocalIndex();
+ return true;
+ }
+
+ private boolean isIndexValid(IndexManifest indexManifest)
Review Comment:
I get the idea, it means the index schema version, it might be changed, we need to rebuild(bootstrap) the index if the old index is not compatible with the new schema. Filed HIVE-30193 as the follow-up task.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4290852591
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/IndexMapping.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * 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.hive.search.mapping;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.config.IndexStoreOptions;
+import org.apache.hive.search.config.InferenceOptions;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.SearchTextSegment;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.analysis.core.KeywordAnalyzer;
+import org.apache.lucene.analysis.miscellaneous.PerFieldAnalyzerWrapper;
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+
+public record IndexMapping(Configuration configuration, Map<String, FieldSchema> fields) {
Review Comment:
This IndexMapping(logical) abstracts away from how the index is organized in the storage(physical), and the reader(Searcher) uses this mapping to validate the input or find the index fields the current query looks for.
Another case is if we want to buy Solr/OpenSearch in the future, we don't need to care about how/what the real field for, as the mapping can be versioned, we just transform this mapping fields received to the layout the storage/persistence layer wants.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r3756187381
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/SearchBackend.java:
##########
@@ -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.
+ */
+
+package org.apache.hive.search.search;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.hadoop.conf.Configuration;
+
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.exception.InitializeException;
+import org.apache.hive.search.exception.SearchException;
+
+/**
+ * Pluggable search storage and query engine. Phase 1 uses embedded Lucene; Phase 3 may add
+ * OpenSearch or Elasticsearch without changing the mapping contract or ingest path.
+ */
+public interface SearchBackend extends Closeable {
+
+ void initialize(Configuration configuration) throws InitializeException, IOException;
+
+ /** Whether this instance can serve search (index opened and bootstrap completed). */
+ boolean isReady() throws IndexNotHealthyException;
+
+ TableSearchResult search(SearchQuery query)
+ throws SearchException, IOException;
+
+ /** Loads stored tables by canonical ids ({@code catalog.db.table}). */
Review Comment:
Can we use this one? We may not.
https://github.com/apache/hive/blob/master/storage-api/src/java/org/apache/hadoop/hive/common/TableName.java
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4355828894
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
Review Comment:
Good catch!
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5289269461
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [298 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [2.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5030373851
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [280 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r3642460331
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/config/IndexStoreOptions.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.hive.search.config;
+
+import java.net.URI;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hive.search.exception.IndexIOException;
+
+public record IndexStoreOptions(Configuration configuration, String indexName) {
+ public static final String LOCAL_PATH = "metastore.index.local.path";
+ public static final String REMOTE_URI = "metastore.index.backup.remote.uri";
+ public static final String MEMORY = "metastore.index.use.memory";
+
+ public static final Path DEFAULT_WORKDIR = Paths.get(System.getProperty("user.dir"), "indexes");
+
+ public Path getLocalPath() {
+ String path = configuration.get(LOCAL_PATH, DEFAULT_WORKDIR.toString());
+ return Path.of(path);
+ }
+
+ public String getRemoteUri() {
+ return configuration.get(REMOTE_URI, "");
+ }
+
+ public boolean hasRemote() {
+ String uri = getRemoteUri();
+ return StringUtils.isNotEmpty(uri);
+ }
+
+ public boolean useMemory() {
+ return configuration.getBoolean(MEMORY, false);
+ }
+
+ public boolean isDistributed() {
+ return hasRemote();
+ }
+
+ public static void validateRemoteUri(String uriText) throws IndexIOException {
+ try {
+ URI.create(uriText);
+ } catch (IllegalArgumentException e) {
+ throw new IndexIOException("invalid store.remote.uri: " + uriText, e);
+ }
+ }
Review Comment:
The thrown error message references a non-existent config key ("store.remote.uri"); this makes troubleshooting harder because the real key is metastore.index.backup.remote.uri.
##########
standalone-metastore/metastore-search/src/test/java/org/apache/hive/search/testutil/InMemoryIndexStateClient.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.hive.search.testutil;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.zip.CRC32;
Review Comment:
The checksum field is named crc32C, but the implementation imports CRC32 (different algorithm). This makes the manifest/checksum semantics misleading and can cause mismatches if any other component expects CRC32C.
This issue also appears on line 99 of the same file.
##########
standalone-metastore/metastore-search/src/test/java/org/apache/hive/search/index/store/TestIndexBackupUtils.java:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.hive.search.index.store;
+
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hive.search.testutil.InMemoryIndexStateClient;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.ByteArrayInputStream;
+import java.util.List;
+import java.util.zip.CRC32;
Review Comment:
This test helper is computing CRC32, but the value is named crc32C. Use CRC32C here to match the manifest field name and the intended semantics.
This issue also appears on line 135 of the same file.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5065600942
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [266 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-5176208167
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [298 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r3758527171
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/SearchBackend.java:
##########
@@ -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.
+ */
+
+package org.apache.hive.search.search;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.hadoop.conf.Configuration;
+
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.exception.InitializeException;
+import org.apache.hive.search.exception.SearchException;
+
+/**
+ * Pluggable search storage and query engine. Phase 1 uses embedded Lucene; Phase 3 may add
+ * OpenSearch or Elasticsearch without changing the mapping contract or ingest path.
+ */
+public interface SearchBackend extends Closeable {
+
+ void initialize(Configuration configuration) throws InitializeException, IOException;
+
+ /** Whether this instance can serve search (index opened and bootstrap completed). */
+ boolean isReady() throws IndexNotHealthyException;
+
+ TableSearchResult search(SearchQuery query)
+ throws SearchException, IOException;
+
+ /** Loads stored tables by canonical ids ({@code catalog.db.table}). */
Review Comment:
ack, will fix it
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4290094889
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
+ this.lastEventId = initialize();
+ this.indexManager.setNid(lastEventId);
+ }
+
+ private boolean rebuildIndex() throws Exception {
+ indexManager.resolveInterruptedRestore();
+ if (isIndexValid(indexManager.readLocalManifest().orElse(null))) {
+ return false;
+ }
+ if (isIndexValid(indexManager.readRemoteManifest().orElse(null))) {
+ if (indexManager.restoreBackup()) {
+ return false;
+ }
+ LOG.warn("Failed to restore index from remote backup; clearing partial local index");
+ indexManager.clearLocalIndex();
+ return true;
+ }
+ indexManager.clearLocalIndex();
+ return true;
+ }
+
+ private boolean isIndexValid(IndexManifest indexManifest)
Review Comment:
I get the idea, it means the index schema version, it might be changed, we need to rebuild(bootstrap) the index if the old index is not compatible with the new schema.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-6194209163
> Clients already connect to Metastore over Thrift
do we support REST client here as well or is that a follow-up?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #6595:
URL: https://github.com/apache/hive/pull/6595#discussion_r4353600170
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/Searcher.java:
##########
@@ -0,0 +1,293 @@
+/*
+ * 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.hive.search.search;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hive.search.config.SearchOptions;
+import org.apache.hive.search.exception.InferenceException;
+import org.apache.hive.search.exception.SearchException;
+import org.apache.hive.search.mapping.FieldSchema;
+import org.apache.hive.search.mapping.IndexMapping;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.mapping.TextFieldSchema;
+import org.apache.hive.search.metastore.MetastoreTableMapper;
+import org.apache.hive.search.metastore.TableBlobCodec;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.inference.Embedder;
+import org.apache.hive.search.inference.EmbedderRegistry;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.queryparser.simple.SimpleQueryParser;
+import org.apache.lucene.search.BayesianScoreEstimator;
+import org.apache.lucene.search.BayesianScoreQuery;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.BoostQuery;
+import org.apache.lucene.search.ConstantScoreQuery;
+import org.apache.lucene.search.DisjunctionMaxQuery;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.KnnFloatVectorQuery;
+import org.apache.lucene.search.LogOddsFusionQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreDoc;
+import org.apache.lucene.search.SearcherManager;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.search.PrefixQuery;
+import org.apache.lucene.search.TopDocs;
+
+public final class Searcher implements AutoCloseable {
+ private final EmbedderRegistry embedderRegistry;
+ private final IndexSearcher indexSearcher;
+ private final SearcherManager searcherManager;
+ private final IndexMapping mapping;
+ private final SearchOptions searchConfig;
+ private final BayesianScoreEstimator.Parameters parameters;
+ private final long processedEventId;
+
+ public Searcher(SearcherManager manager,
+ IndexManager indexManager,
+ EmbedderRegistry registry,
+ SearchOptions searchConfig,
+ BayesianScoreEstimator.Parameters parameters) throws IOException {
+ this.searcherManager = manager;
+ this.processedEventId = indexManager.getProcessedEventId();
+ this.indexSearcher = manager.acquire();
+ this.mapping = indexManager.mapping();
+ this.searchConfig = searchConfig;
+ this.parameters = parameters;
+ this.embedderRegistry = Objects.requireNonNull(registry, "Embedder registry");
+ }
+
+ public TableSearchResult search(SearchQuery request)
+ throws SearchException, IOException {
+ if (StringUtils.isBlank(request.body().queryText())) {
+ throw new SearchException("query text is required");
+ }
+ Query filter = compileScopeFilter(request.catalogName(), request.databaseName());
+ int size = request.limit() > 0 ? request.limit() : searchConfig.getDefaultLimit();
+ Query query =
+ switch (request.body()) {
+ case MatchQuery match -> {
+ Query matchQuery = compileMatchQuery(match.resolve(mapping));
+ if (filter == null) {
+ yield matchQuery;
+ }
+ yield new BooleanQuery.Builder()
+ .add(matchQuery, BooleanClause.Occur.MUST)
+ .add(filter, BooleanClause.Occur.FILTER)
+ .build();
+ }
+ case SemanticQuery semantic -> compileSemanticQuery(semantic.resolve(mapping), size, filter);
+ case HybridQuery hybrid -> compileFusionQuery(hybrid.resolve(mapping), size, filter);
+ };
+ TopDocs topDocs = indexSearcher.search(query, size);
+ List<TableSearchHit> hits = new ArrayList<>();
+ for (ScoreDoc scoreDoc : topDocs.scoreDocs) {
+ hits.add(readHit(scoreDoc));
+ }
+ return new TableSearchResult(hits, topDocs.totalHits.value(), processedEventId);
+ }
+
+ public TableSearchResult loadTables(List<TableName> tableNames) throws SearchException, IOException {
+ if (tableNames == null || tableNames.isEmpty()) {
+ return new TableSearchResult(List.of(), 0, processedEventId);
+ }
+ List<String> ids = new ArrayList<>(tableNames.size());
+ for (TableName table : tableNames) {
+ ids.add(MetastoreTableMapper.tableId(table));
+ }
+ BooleanQuery.Builder builder = new BooleanQuery.Builder();
+ for (String id : ids) {
+ builder.add(
+ new TermQuery(new Term("_id" + TableDocument.FILTER_SUFFIX, id)),
+ BooleanClause.Occur.SHOULD);
+ }
+ TopDocs hits = indexSearcher.search(builder.build(), ids.size());
+ List<TableSearchHit> searchHits = new ArrayList<>(hits.scoreDocs.length);
+ for (ScoreDoc scoreDoc : hits.scoreDocs) {
+ searchHits.add(readHit(scoreDoc));
+ }
+ return new TableSearchResult(searchHits, hits.totalHits.value(), processedEventId);
+ }
+
+ private Query compileFusionQuery(HybridQuery query, int semanticK, Query filter)
+ throws SearchException {
+ Query matchQuery = new BayesianScoreQuery(
Review Comment:
The scope filter only reaches the semantic branch. Hybrid lexical hits can escape the requested catalog/database right?
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
+ this.lastEventId = initialize();
+ this.indexManager.setNid(lastEventId);
+ }
+
+ private boolean rebuildIndex() throws Exception {
+ indexManager.resolveInterruptedRestore();
+ if (isIndexValid(indexManager.readLocalManifest().orElse(null))) {
+ return false;
+ }
+ if (isIndexValid(indexManager.readRemoteManifest().orElse(null))) {
+ if (indexManager.restoreBackup()) {
+ return false;
+ }
+ LOG.warn("Failed to restore index from remote backup; clearing partial local index");
+ indexManager.clearLocalIndex();
+ return true;
+ }
+ indexManager.clearLocalIndex();
+ return true;
+ }
+
+ private boolean isIndexValid(IndexManifest indexManifest)
+ throws Exception {
+ if (indexManifest == null) {
+ return false;
+ }
+ String current = indexManager.mapping().inference().embedderName();
+ String embedderName = indexManifest.embedder();
+ if (!current.equals(embedderName)) {
+ return false;
+ }
+ long notificationId = indexManifest.lastEventId();
+ return canCatchUp(notificationId);
+ }
+
+ private boolean canCatchUp(long notificationId) throws Exception {
+ if (notificationId < 0) {
+ return false;
+ }
+ try {
+ NotificationEventRequest request = new NotificationEventRequest(notificationId);
+ request.setMaxEvents(10);
+ client.getNextNotification(request, false, null);
+ return true;
+ } catch (IllegalStateException ignored) {
+ LOG.debug("Current index lags too behind, will rebuild");
+ }
+ return false;
+ }
+
+ private long initialize() throws Exception {
+ boolean rebuild = rebuildIndex();
+ if (rebuild && !cluster.isLeader()) {
+ long notificationId = waitForRemoteBackup();
+ indexer.initialize();
+ if (notificationId >= 0) {
+ return notificationId;
+ }
+ return rebuildLeaderIndex();
+ }
+ indexer.initialize();
+ if (rebuild) {
+ return rebuildLeaderIndex();
+ }
+ return indexManager.readLocalManifest().get().lastEventId();
+ }
+
+ private long rebuildLeaderIndex() throws Exception {
+ indexManager.clearRemoteIndex();
+ long notificationId = client.getCurrentNotificationEventId().getEventId();
+ new BootstrapIndexer(
+ indexManager,
+ indexer,
+ client,
+ shareBootstrapFetchClient).run(notificationId);
+ return notificationId;
+ }
+
+ public void start() throws Exception {
+ handler.start(lastEventId);
Review Comment:
Please start the flush listener first. The poller can process an event before line 153 resets its checkpoint.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
+ this.lastEventId = initialize();
+ this.indexManager.setNid(lastEventId);
+ }
+
+ private boolean rebuildIndex() throws Exception {
+ indexManager.resolveInterruptedRestore();
+ if (isIndexValid(indexManager.readLocalManifest().orElse(null))) {
+ return false;
+ }
+ if (isIndexValid(indexManager.readRemoteManifest().orElse(null))) {
+ if (indexManager.restoreBackup()) {
+ return false;
+ }
+ LOG.warn("Failed to restore index from remote backup; clearing partial local index");
+ indexManager.clearLocalIndex();
+ return true;
+ }
+ indexManager.clearLocalIndex();
+ return true;
+ }
+
+ private boolean isIndexValid(IndexManifest indexManifest)
+ throws Exception {
+ if (indexManifest == null) {
+ return false;
+ }
+ String current = indexManager.mapping().inference().embedderName();
+ String embedderName = indexManifest.embedder();
+ if (!current.equals(embedderName)) {
+ return false;
+ }
+ long notificationId = indexManifest.lastEventId();
+ return canCatchUp(notificationId);
+ }
+
+ private boolean canCatchUp(long notificationId) throws Exception {
+ if (notificationId < 0) {
+ return false;
+ }
+ try {
+ NotificationEventRequest request = new NotificationEventRequest(notificationId);
+ request.setMaxEvents(10);
+ client.getNextNotification(request, false, null);
+ return true;
+ } catch (IllegalStateException ignored) {
+ LOG.debug("Current index lags too behind, will rebuild");
+ }
+ return false;
+ }
+
+ private long initialize() throws Exception {
+ boolean rebuild = rebuildIndex();
+ if (rebuild && !cluster.isLeader()) {
+ long notificationId = waitForRemoteBackup();
+ indexer.initialize();
+ if (notificationId >= 0) {
+ return notificationId;
+ }
+ return rebuildLeaderIndex();
+ }
+ indexer.initialize();
+ if (rebuild) {
+ return rebuildLeaderIndex();
+ }
+ return indexManager.readLocalManifest().get().lastEventId();
+ }
+
+ private long rebuildLeaderIndex() throws Exception {
+ indexManager.clearRemoteIndex();
+ long notificationId = client.getCurrentNotificationEventId().getEventId();
+ new BootstrapIndexer(
+ indexManager,
+ indexer,
+ client,
+ shareBootstrapFetchClient).run(notificationId);
+ return notificationId;
+ }
+
+ public void start() throws Exception {
+ handler.start(lastEventId);
+ flushIndexListener.start(lastEventId);
+ }
+
+ /** Package-private for integration tests. */
+ int pollEvents(int count) throws IndexNotHealthyException {
+ return handler.getNextMetastoreEvents(count);
+ }
+
+ /** Package-private for integration tests. */
+ void flushCheckpoint() throws IOException {
+ try {
+ indexer.flush(client.getCurrentNotificationEventId().getEventId(), true);
+ } catch (Exception e) {
+ if (e instanceof IOException ioException) {
+ throw ioException;
+ }
+ throw new IOException("Failed to flush index checkpoint", e);
+ }
+ }
+
+ /** Package-private for integration tests. */
+ void syncBackup() throws IOException {
+ indexer.syncBackup();
+ }
+
+ /**
+ * Wait for a follower-ready remote backup and restore it before the IndexWriter is opened.
+ *
+ * @return restored notification id, or -1 if this instance became leader while waiting
+ */
+ private long waitForRemoteBackup() throws Exception {
+ String current = indexManager.mapping().inference().embedderName();
+ for (; !cluster.isLeader(); Thread.sleep(3000)) {
+ Optional<IndexManifest> indexManifest = indexManager.readRemoteManifest();
+ if (indexManifest.isEmpty()) {
+ continue;
+ }
+ IndexManifest manifest = indexManifest.get();
+ if (!current.equals(manifest.embedder())) {
+ LOG.debug("Remote index model {} does not match configured model {}, waiting",
+ manifest.embedder(), current);
+ continue;
+ }
+ long notificationId = manifest.lastEventId();
+ if (notificationId > 0 && canCatchUp(notificationId)) {
Review Comment:
1st hypothetical case: Event ID 0 is valid for an empty metastore; this condition leaves followers waiting forever.
##########
standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/metastore/MetastoreIndexer.java:
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.hive.search.metastore;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.DatabaseName;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
+import org.apache.hadoop.hive.metastore.api.NotificationEventRequest;
+import org.apache.hadoop.hive.metastore.leader.LeaderElection;
+import org.apache.hive.search.mapping.TableDocument;
+import org.apache.hive.search.exception.IndexNotHealthyException;
+import org.apache.hive.search.config.IndexOptions;
+import org.apache.hive.search.index.Indexer;
+import org.apache.hive.search.index.IndexManager;
+import org.apache.hive.search.index.store.IndexManifest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class MetastoreIndexer implements Closeable {
+ private static final Logger LOG = LoggerFactory.getLogger(MetastoreIndexer.class);
+
+ private final IMetaStoreClient client;
+ private final long lastEventId;
+ private final Indexer indexer;
+ private final MetastoreCluster cluster;
+ private final MetastoreEventHandler handler;
+ private final IndexManager indexManager;
+ private final FlushIndexListener flushIndexListener;
+ private final boolean shareBootstrapFetchClient;
+
+ public MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer)
+ throws Exception {
+ this(configuration, indexManager, indexer,
+ RetryingMetaStoreClient.getProxy(configuration, true), false);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client) throws Exception {
+ this(configuration, indexManager, indexer, client, true);
+ }
+
+ MetastoreIndexer(Configuration configuration, IndexManager indexManager, Indexer indexer,
+ IMetaStoreClient client, boolean shareBootstrapFetchClient) throws Exception {
+ this.client = client;
+ this.shareBootstrapFetchClient = shareBootstrapFetchClient;
+ this.indexer = indexer;
+ this.indexManager = indexManager;
+ this.flushIndexListener = new FlushIndexListener(configuration);
+ this.handler = new MetastoreEventHandler(configuration, client);
+ this.handler.addListeners(flushIndexListener, indexManager);
+ this.cluster = new MetastoreCluster(configuration, flushIndexListener);
Review Comment:
2nd hypothetical case: If initialization fails, the constructor leaks this client and leader election because the object never becomes closeable.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-29681: Introduce contextual table search [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6595:
URL: https://github.com/apache/hive/pull/6595#issuecomment-6284420068
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595) **Quality Gate passed**
Issues
 [316 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6595&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6595&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6595&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6595)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org