Skip to content

Use upper-case HTS predicates to match the functional index - #758

Merged
ruolin59 merged 2 commits into
mainfrom
rufan/hts-upper-index-hotfix
Sep 19, 2026
Merged

ruolin59 merged 2 commits into
mainfrom
rufan/hts-upper-index-hotfix

Conversation

@ruolin59

@ruolin59 ruolin59 commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

Production incident

HTS in prod-ltx1 saturated and began refusing TCP connections at ~11:35 PDT on 2026-09-18, breaking Airflow partition sensors with Connection refused ... :4768.

user_table_row has ~513,000 rows and a functional index:

idx_user_table_upper_db_table ON (upper(database_id), upper(table_id))

A functional index matches only the exact expression, so handwritten lower(...) cannot use it. getUserTable runs at roughly 6,000 QPS; a full scan per request exhausted MySQL connections and HTS worker threads, and pods stopped accepting connections.

What regressed

Before eca3ca3 (#696), getUserTable went through findById(key), a default method delegating to the Spring-derived findByDatabaseIdIgnoreCaseAndTableIdIgnoreCase. Spring Data emits upper(...) for IgnoreCase (JpaQueryCreator$PredicateBuilder.upperIfIgnoreCase), so it matched the index. That is why the index is on upper.

#696 replaced that call with a new explicit @Query using handwritten lower(...), following the file's existing convention for hand-written queries. The method name still reads like the derived finder it displaced, which is how it passed review.

The six pre-existing lower(...) queries were all on cold paths — filters, search, rename. They had been scanning for a long time without anyone noticing, because a 500ms scan at low QPS is invisible. Moving the hot path onto that convention is what turned it into an outage.

The fix

lower( → upper( at all 30 token occurrences across the 15 comparison sites in UserTableHtsJdbcRepository, both sides of every comparison. Nothing else.

Production EXPLAIN ANALYZE, measured on the real table:

plan rows time
before Table scan on user_table_row 513,418 ~495ms
after Index lookup using idx_user_table_upper_db_table 1 ~0.041ms

The entity_type predicate is not the cause. In the fixed plan it is demoted to a cheap residual filter above the index lookup. It is implicated only because it arrived in the same commit.

Why this is semantically safe

Production collation is confirmed utf8mb4_0900_ai_ci, which is case- and accent-insensitive. Under it, lower(a) = lower(b), upper(a) = upper(b) and a = b are equivalent. The wrapping affects index eligibility, not which rows match.

At the sole LIKE site both column and pattern fold with the same function, and %, _ and escapes are non-alphabetic so upper() leaves them byte-identical — wildcard semantics are unchanged.

Tests

Two tests added to HtsRepositoryTest, covering cross-case matching for the filter and LIKE families. Those families were previously exercised only with same-case data, so a botched substitution could have slipped through; point reads, rename and deletes already had cross-case coverage.

These tests pass both before and after the change, and that is deliberate. There is no red phase because the change is behaviour-neutral by design.

What the tests prove: behaviour preservation across case for the affected query families.

What they cannot prove: index selection, scan avoidance, or latency. Tests run against H2 in MySQL mode, which has no functional indexes and no meaningful planner. The performance claim rests solely on the production EXPLAIN ANALYZE above.

Suites pass on JDK 11: housetables 406, common 12, zero failures, errors or skips.

Deliberately out of scope

SoftDeletedUserTableHtsJdbcRepository has 24 lower( tokens across 12 lines on soft_deleted_user_table_row. That is a different physical table whose indexes are unconfirmed — flipping it blind could be a no-op or a pessimisation. Its paths are cold (restore, purge, querying deleted tables). Needs its own SHOW INDEX before anyone touches it.

The schema record is corrected in this PR. services/housetables/ddl/0000__baseline.sql previously recorded only PRIMARY KEY (database_id, table_id) for user_table_row, and its own header warned that a derived definition "cannot capture secondary indexes". So the index this fix depends on was documented nowhere, and anyone reconstructing the table from that file would have reintroduced this outage.

user_table_row and soft_deleted_user_table_row are now transcribed from production SHOW CREATE TABLE. For user_table_row that closed more than the index: database_id/table_id were recorded as varchar(128) but are varchar(255), metadata_location as varchar(512) but is varchar(255), version as NOT NULL but is nullable, last_modified_time was recorded but does not exist, and table_version and deleted_ts exist but were absent. Engine, charset and collation were missing from both tables. soft_deleted_user_table_row has no secondary index, and that is now recorded as the real state rather than an omission.

job_row and table_toggle_rule are deliberately untouched — no production output was available for them, and guessing would recreate exactly the failure this PR is fixing. The header now says which two tables are verified and which two are not.

This also explains why no local or containerised MySQL could have caught the regression: the oh-only-mysql recipe bootstraps from this DDL, so a local database had no functional index and lower() versus upper() was indistinguishable there.

Replace lower() with upper() on both sides of handwritten JPQL
comparisons in UserTableHtsJdbcRepository, preserving entity-type
filtering and case-insensitive filter and LIKE behavior.

The idx_user_table_upper_db_table index on
(upper(database_id), upper(table_id)) cannot match lower() expressions.
At roughly 6,000 QPS, production EXPLAIN ANALYZE measured 513,418
scanned rows / ~495ms before the change and an index lookup of
1 row / ~0.041ms with upper().

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@abhisheknath2011

Copy link
Copy Markdown
Member

Production incident

HTS in prod-ltx1 saturated and began refusing TCP connections at ~11:35 PDT on 2026-09-18, breaking Airflow partition sensors with Connection refused ... :4768.

user_table_row has ~513,000 rows and a functional index:

idx_user_table_upper_db_table ON (upper(database_id), upper(table_id))

A functional index matches only the exact expression, so handwritten lower(...) cannot use it. getUserTable runs at roughly 6,000 QPS; a full scan per request exhausted MySQL connections and HTS worker threads, and pods stopped accepting connections.

What regressed

Before eca3ca3 (#696), getUserTable went through findById(key), a default method delegating to the Spring-derived findByDatabaseIdIgnoreCaseAndTableIdIgnoreCase. Spring Data emits upper(...) for IgnoreCase (JpaQueryCreator$PredicateBuilder.upperIfIgnoreCase), so it matched the index. That is why the index is on upper.

#696 replaced that call with a new explicit @Query using handwritten lower(...), following the file's existing convention for hand-written queries. The method name still reads like the derived finder it displaced, which is how it passed review.

The six pre-existing lower(...) queries were all on cold paths — filters, search, rename. They had been scanning for a long time without anyone noticing, because a 500ms scan at low QPS is invisible. Moving the hot path onto that convention is what turned it into an outage.

The fix

lower( → upper( at all 30 token occurrences across the 15 comparison sites in UserTableHtsJdbcRepository, both sides of every comparison. Nothing else.

Production EXPLAIN ANALYZE, measured on the real table:

plan rows time
before Table scan on user_table_row 513,418 ~495ms
after Index lookup using idx_user_table_upper_db_table 1 ~0.041ms
The entity_type predicate is not the cause. In the fixed plan it is demoted to a cheap residual filter above the index lookup. It is implicated only because it arrived in the same commit.

Why this is semantically safe

Production collation is confirmed utf8mb4_0900_ai_ci, which is case- and accent-insensitive. Under it, lower(a) = lower(b), upper(a) = upper(b) and a = b are equivalent. The wrapping affects index eligibility, not which rows match.

At the sole LIKE site both column and pattern fold with the same function, and %, _ and escapes are non-alphabetic so upper() leaves them byte-identical — wildcard semantics are unchanged.

Tests

Two tests added to HtsRepositoryTest, covering cross-case matching for the filter and LIKE families. Those families were previously exercised only with same-case data, so a botched substitution could have slipped through; point reads, rename and deletes already had cross-case coverage.

These tests pass both before and after the change, and that is deliberate. There is no red phase because the change is behaviour-neutral by design.

What the tests prove: behaviour preservation across case for the affected query families.

What they cannot prove: index selection, scan avoidance, or latency. Tests run against H2 in MySQL mode, which has no functional indexes and no meaningful planner. The performance claim rests solely on the production EXPLAIN ANALYZE above.

Suites pass on JDK 11: housetables 406, common 12, zero failures, errors or skips.

Deliberately out of scope

SoftDeletedUserTableHtsJdbcRepository has 24 lower( tokens across 12 lines on soft_deleted_user_table_row. That is a different physical table whose indexes are unconfirmed — flipping it blind could be a no-op or a pessimisation. Its paths are cold (restore, purge, querying deleted tables). Needs its own SHOW INDEX before anyone touches it.

The index is not recorded in this repository. services/housetables/ddl/0000__baseline.sql defines only PRIMARY KEY (database_id, table_id) for user_table_row, and its own header notes that a derived definition "cannot capture secondary indexes". So this fix depends on an index the schema record does not document, and anyone reconstructing the table from that DDL would reintroduce this outage. It also explains why no local or containerised MySQL could have caught this: the oh-only-mysql recipe bootstraps from that DDL, so a local database has no functional index and lower() versus upper() is indistinguishable there. Worth a follow-up to record the real SHOW CREATE TABLE.

Thanks for the fix @ruolin59. Do we need index on the entity_type column?

@ruolin59

Copy link
Copy Markdown
Collaborator Author

@abhisheknath2011

Do we need index on the entity_type column?

no, that would be the incorrect thing to do here. the issue isn't the lack of index on entity_type, but the fact that lower() calls caused the queries to stop using the existing indices on the dbid and tblid columns. This is the reason I'm still asking for the SHOW CREATE TABLE results for the 2 mysql tables. If we had the proper table definitions this whole issue would have been avoided

Replace the derived, unverified definitions of user_table_row and
soft_deleted_user_table_row with their production SHOW CREATE TABLE
output (minus the entity_type column, which is added by 0001).

For user_table_row this closes several gaps that a derived definition
could not capture: the idx_user_table_upper_db_table functional index
on upper(database_id)/upper(table_id) that the service's query plan
depends on, the InnoDB engine and utf8mb4 / utf8mb4_0900_ai_ci charset
and collation, and the physical column order. It also fixes the
column types (database_id, table_id to varchar(255), metadata_location
to varchar(255)), makes version nullable, adds the table_version and
deleted_ts columns, and drops last_modified_time, which production
does not have.

soft_deleted_user_table_row gains its InnoDB engine and utf8mb4 /
utf8mb4_0900_ai_ci charset and collation; it has no secondary index.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@abhisheknath2011 abhisheknath2011 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@ruolin59
ruolin59 merged commit 2cdbb83 into main Sep 19, 2026
1 check passed
@ruolin59
ruolin59 deleted the rufan/hts-upper-index-hotfix branch September 19, 2026 00:52
teamurko pushed a commit that referenced this pull request Sep 22, 2026
#758) (#761)

## Summary

Reverts two related commits:

- #696 "Add entityType discriminator and table-scoped HTS queries"
- #758 "Use upper-case HTS predicates to match the functional index"

#758 was a hotfix for a production incident (HTS connection saturation)
caused by
`getUserTable` scanning the full `user_table_row` table instead of using
the
`idx_user_table_upper_db_table` functional index, a regression
introduced by #696's
handwritten `lower(...)` predicates.

This PR reverts both changes back to the pre-#696 state, restoring the
original
`findByDatabaseIdIgnoreCaseAndTableIdIgnoreCase`-based query path (which
naturally
matched the functional index) and removing the `entity_type`
discriminator column,
`EntityType` enum, and related JDBC/API/test surface added by #696.

A follow-up PR will reintroduce both changes together, combined into a
single
commit, so the entityType feature and its required index-compatible
predicate fix
land atomically.

## Test plan

- `./gradlew :services:housetables:test :services:common:test` passes.
- `./gradlew spotlessCheck` passes (run with `-x CopyGitHooksTask`, a
pre-existing
worktree-incompatibility in the git-hooks Gradle task, unrelated to this
change).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ruolin59 added a commit to ruolin59/openhouse that referenced this pull request Sep 22, 2026
…#696, linkedin#758 combined)

Combines two previously-separate commits into one so the entityType
discriminator feature and its required index-compatible predicate lands
atomically:

- Add entityType discriminator and table-scoped HTS queries (originally linkedin#696)
- Use upper-case HTS predicates to match the functional index (originally linkedin#758,
  a hotfix for a production incident caused by linkedin#696's lower(...) predicates
  bypassing idx_user_table_upper_db_table)

See the reverted PR (linkedin#761 revert of linkedin#696/linkedin#758) for background on why these two
were split apart and are now being reintroduced together.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ruolin59 added a commit to ruolin59/openhouse that referenced this pull request Sep 22, 2026
…fix (linkedin#696, linkedin#758)" (linkedin#761)

This reverts the revert in linkedin#761, restoring linkedin#696 and linkedin#758 combined into a
single commit:

- Add entityType discriminator and table-scoped HTS queries (originally linkedin#696)
- Use upper-case HTS predicates to match the functional index (originally
  linkedin#758, a hotfix for the production incident caused by linkedin#696's handwritten
  lower(...) predicates bypassing the idx_user_table_upper_db_table
  functional index)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants