Add new IndexLogEntryTag to avoid duplicate calculation in getCandidateIndexes

[sezruby]

What is the context for this pull request?

• Tracking Issue: n/a
• Parent Issue: n/a
• Dependencies: n/a

What changes were proposed in this pull request?

Introduce new tags to avoid duplicate operation while applying rules.

• SIGNATURE_MATCHED tag keeps the result of def signatureValid for each relation & indexes, and reuse if exists. Since there's no config to affect the result, it can be reused without any check.
• IS_HYBRIDSCAN_CANDIDATE' tag keeps the result of def isHybridScanCandidate` for each relation & indexes. If some hybrid scan related configs are changed, the result can be changed. So before using this tag, reset the tag values if related configs are changed.
• related configs
  • "spark.hyperspace.index.hybridscan.maxDeletedRatio"
  • "spark.hyperspace.index.hybridscan.maxAppendedRatio"
• HYBRIDSCAN_RELATED_CONFIGS keeps the related config values.

Test results

Test data

• 100k chunk lineitem table
• 1 non-candidate index (but with applicable column schema; so that we could measure the getCandidateIndex performance)
• unit: ms

Test scripts

val linetable = spark.read.parquet(tableName)
val filter = linetable.filter(linetable("l_orderkey") isin (1234,12341234, 123456)).select("l_orderkey")
measure(filter.queryExecution.optimizedPlan)
val filter = linetable.filter(linetable("l_orderkey") isin (1234,12341234, 123456)).select("l_orderkey")
measure(filter.queryExecution.optimizedPlan)
val filter = linetable.filter(linetable("l_orderkey") isin (1234,12341234, 123456)).select("l_orderkey")
measure(filter.queryExecution.optimizedPlan)

Result w/o this change:

// Non Hybrid Scan
ilter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 1533
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 1359
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 1347

// Hybrid Scan
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 3505
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 1314
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 1392

Result with this change

// Non Hybrid Scan
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 1690
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 11
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 10

// Hybrid Scan
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 1190
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 313 // cost from constructing the list of FileInfo for the given plan
filter: org.apache.spark.sql.DataFrame = [l_orderkey: bigint]
duration: 306

Does this PR introduce any user-facing change?

No, it's an internal optimization.

How was this patch tested?

existing tests will cover these changes.

[imback82]

Result (only this PR - including InMemoryFileIndex cost)

So, what's the summary? I wasn't sure which number to look at to see the gain. Also, is the duration in milliseconds?

[sezruby]

@imback82 I updated the test results with 2 different binaries.
The previous result was incorrect because of the cost for cachedIndexCollectionManager.

[imback82]

Cool, the result makes more sense now. I will get to the review this weekend. @apoorvedave1 / @pirz can you review this PR meanwhile? Thanks!

[imback82]

General approach seems fine to me.

[imback82]

nit: prefer using map or foreach for this pattern taggedConfigs.isEmpty || taggedConfigs.get:

      index.getTagValue(plan, HYBRIDSCAN_CONFIG_CAPTURE).foreach { taggedConfigs =>
        if (taggedConfigs.equals(curConfigs)) {
          // Need to reset cached tags as these config changes can change the result.
          index.unsetTagValue(plan, IS_HYBRIDSCAN_CANDIDATE)
          index.setTagValue(plan, HYBRIDSCAN_CONFIG_CAPTURE, curConfigs)
        }
      }

[imback82]

• "spark.hyperspace.index.hybridscan.delete.enabled"
• "spark.hyperspace.index.hybridscan.delete.maxNumDeletedFiles"

Can you update these, which are not in the code base anymore?

[imback82]

Not related to this PR, but we need to think about refactoring RuleUtils.scala. This is already two levels deep (getCandidateIndexes -> isHybridScanCandidate -> prepareHybridScanCandidateSelection).

[imback82]

• "spark.hyperspace.index.hybridscan.delete.enabled"
• "spark.hyperspace.index.hybridscan.delete.maxNumDeletedFiles"

Can you update these, which are not in the code base anymore?

Btw, this comment was regarding the PR description.

[imback82]

LGTM (few minor comments), thanks @sezruby!

[imback82]

@sezruby Can you update the PR description? It is now out of sync with code. Thanks!