[HUDI-7563] Add support to drop index using sql #11951
Conversation
codope
force-pushed
the
drop-index-hudi-7563
branch
from
5fc8434
to
0ea02b9
Compare
...ommon/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java
Outdated
Show resolved
Hide resolved
...ommon/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java
Outdated
Show resolved
Hide resolved
...ommon/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java
Outdated
Show resolved
Hide resolved
| public static HoodieSparkIndexClient getInstance(SparkSession sparkSession) { | ||
| if (_instance == null) { | ||
| synchronized (HoodieSparkFunctionalIndexClient.class) { | ||
| synchronized (HoodieSparkIndexClient.class) { | ||
| if (_instance == null) { | ||
| _instance = new HoodieSparkFunctionalIndexClient(sparkSession); | ||
| _instance = new HoodieSparkIndexClient(sparkSession); |
There was a problem hiding this comment.
This logic is a bit weird for concurrency control. We should revisit it later.
There was a problem hiding this comment.
This is the usual double-checked locking pattern. Probably you are concerned about memory visibility issues. The _instance could be non-null and yet not fully constructed when the first if (_instance == null) check passes, but before the object is fully initialized. However, note that _instance is declared as volatile which ensures that any writes to _instance are visible to all threads immediately and prevents the reordering of instructions during construction.
codope
force-pushed
the
drop-index-hudi-7563
branch
from
0ea02b9
to
235756d
Compare
| if (!indexExists(metaClient, indexName)) { | ||
| if (ignoreIfNotExists) { | ||
| return; | ||
| } else { | ||
| throw new HoodieFunctionalIndexException("Index does not exist: " + indexName); | ||
| } | ||
| } | ||
|
|
||
| LOG.info("Dropping index {}", indexName); | ||
| HoodieIndexDefinition indexDefinition = metaClient.getIndexMetadata().get().getIndexDefinitions().get(indexName); | ||
| try (SparkRDDWriteClient writeClient = HoodieCLIUtils.createHoodieWriteClient( | ||
| sparkSession, metaClient.getBasePath().toString(), mapAsScalaImmutableMap(buildWriteConfig(metaClient, indexDefinition)), toScalaOption(Option.empty()))) { | ||
| writeClient.dropIndex(Collections.singletonList(indexName)); |
There was a problem hiding this comment.
For future reference, this logic does not have much specifics to engine itself, so it can be abstracted to the index client by plugging in the engine-specific write client.
There was a problem hiding this comment.
Don't we need the engine-specific write client to call the base API BaseHoodieWriteClient.dropIndex?
| HoodieSparkIndexClient.getInstance(sparkSession).drop(metaClient, indexName, ignoreIfNotExists) | ||
| } catch { | ||
| case _: IllegalArgumentException => | ||
| SecondaryIndexManager.getInstance().drop(metaClient, indexName, ignoreIfNotExists) |
There was a problem hiding this comment.
This is legacy code due to incomplete RFC-52. SecondaryIndexManager was introduced in RFC-52 but it's just a wrapper code and does not really manage any index underneath. From RFC, it's supposed to support index built on third party libraries such as Lucene. But, we have not yet added any support so far. In my opinion, we should remove all that code. Just keeping it here to make some tests pass. If you agree, I can take the cleanup as a followup later.
...asource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestSecondaryIndexPruning.scala
Outdated
Show resolved
Hide resolved
.../hudi-spark/src/test/scala/org/apache/spark/sql/hudi/command/index/TestFunctionalIndex.scala
Outdated
Show resolved
Hide resolved
.../hudi-spark/src/test/scala/org/apache/spark/sql/hudi/command/index/TestFunctionalIndex.scala
Outdated
Show resolved
Hide resolved
codope
force-pushed
the
drop-index-hudi-7563
branch
from
235756d
to
e8a0631
Compare
Change Logs
Impact
Users can now drop index using SQL
Risk level (write none, low medium or high below)
low
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist