-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add db-tune ops subcommand + production RDS runbook #27890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
7611649
a719d47
8b3c3e2
f5a945a
28cdd7d
2d9f047
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| /* | ||
| * Copyright 2026 Collate | ||
| * Licensed 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.openmetadata.it.tests; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| import java.util.Map; | ||
| import org.jdbi.v3.core.Jdbi; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.parallel.Execution; | ||
| import org.junit.jupiter.api.parallel.ExecutionMode; | ||
| import org.openmetadata.it.bootstrap.TestSuiteBootstrap; | ||
| import org.openmetadata.service.jdbi3.locator.ConnectionType; | ||
| import org.openmetadata.service.util.dbtune.Action; | ||
| import org.openmetadata.service.util.dbtune.AutoTuner; | ||
| import org.openmetadata.service.util.dbtune.DbTuneResult; | ||
| import org.openmetadata.service.util.dbtune.MysqlAutoTuner; | ||
| import org.openmetadata.service.util.dbtune.PostgresAutoTuner; | ||
| import org.openmetadata.service.util.dbtune.TableRecommendation; | ||
|
|
||
| /** | ||
| * End-to-end tests for {@link AutoTuner} against the live Testcontainers database. | ||
| * | ||
| * <p>The read-only tests ({@link #analyzeReturnsRecommendationsForKnownTables}, {@link | ||
| * #dryRunDoesNotMutateReloptions}) run against the real catalog tables that the IT bootstrap | ||
| * created via migrations. | ||
| * | ||
| * <p>Tests that exercise the write path ({@link #applyExecutesAndIsIdempotent}, {@link | ||
| * #analyzeOneRunsOnIsolatedTable}) deliberately use a private throwaway table — never a real | ||
| * catalog table. Reason: {@code ALTER TABLE} on a shared production table bumps MySQL's per-table | ||
| * metadata version, which invalidates JDBC prepared-statement caches across the whole | ||
| * Testcontainer. When that table has a {@code JSON} column (e.g. {@code entity_relationship}), the | ||
| * driver's re-prepared metadata sometimes returns the column type as {@code VARBINARY}, and | ||
| * subsequent {@code INSERT} statements fail with {@code "Cannot create a JSON value from a string | ||
| * with CHARACTER SET 'binary'"}. We saw this break {@code GlossaryTermRelationsIT}, | ||
| * {@code DomainResourceIT}, and the lineage ITs in CI when an earlier version of this test applied | ||
| * settings to {@code entity_relationship}. The recommendations themselves are sound — the IT just | ||
| * cannot afford the side effect on a shared DB. | ||
| * | ||
| * <p>Sequential because {@code @BeforeEach} / {@code @AfterEach} create and drop the same isolated | ||
| * table by name; concurrent execution would race. | ||
| */ | ||
| @Execution(ExecutionMode.SAME_THREAD) | ||
| class DbTuneIT { | ||
|
|
||
| /** Table created and dropped per test — never a catalog table. Safe blast radius. */ | ||
| private static final String ISOLATED_TABLE = "dbtune_it_isolated_table"; | ||
|
|
||
| /** A real catalog table used only by the read-only tests to assert against the live schema. */ | ||
| private static final String READ_ONLY_PROBE_TABLE = "entity_relationship"; | ||
|
|
||
| @BeforeEach | ||
| void createIsolatedTable() { | ||
| Jdbi jdbi = TestSuiteBootstrap.getJdbi(); | ||
| ConnectionType connType = currentConnectionType(); | ||
| jdbi.useHandle( | ||
| handle -> { | ||
| handle.execute("DROP TABLE IF EXISTS " + quoteIdent(connType, ISOLATED_TABLE)); | ||
| if (connType == ConnectionType.POSTGRES) { | ||
| handle.execute( | ||
| "CREATE TABLE " + quoteIdent(connType, ISOLATED_TABLE) + " (id INT PRIMARY KEY)"); | ||
| } else { | ||
| handle.execute( | ||
| "CREATE TABLE " | ||
| + quoteIdent(connType, ISOLATED_TABLE) | ||
| + " (id INT PRIMARY KEY) ENGINE=InnoDB"); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| @AfterEach | ||
| void dropIsolatedTable() { | ||
| Jdbi jdbi = TestSuiteBootstrap.getJdbi(); | ||
| ConnectionType connType = currentConnectionType(); | ||
| jdbi.useHandle( | ||
| handle -> handle.execute("DROP TABLE IF EXISTS " + quoteIdent(connType, ISOLATED_TABLE))); | ||
| } | ||
|
|
||
| @Test | ||
| void analyzeReturnsRecommendationsForKnownTables() { | ||
| AutoTuner tuner = currentTuner(); | ||
| Jdbi jdbi = TestSuiteBootstrap.getJdbi(); | ||
|
|
||
| DbTuneResult result = jdbi.withHandle(tuner::analyze); | ||
|
|
||
| assertNotNull(result); | ||
| assertNotNull(result.engineVersion()); | ||
| assertFalse(result.tableRecommendations().isEmpty(), "Expected at least one recommendation"); | ||
| assertTrue( | ||
| result.tableRecommendations().stream() | ||
| .anyMatch(r -> READ_ONLY_PROBE_TABLE.equals(r.tableName())), | ||
| READ_ONLY_PROBE_TABLE + " should be in the recommendations"); | ||
| } | ||
|
|
||
| @Test | ||
| void applyExecutesAndIsIdempotent() { | ||
| AutoTuner tuner = currentTuner(); | ||
| Jdbi jdbi = TestSuiteBootstrap.getJdbi(); | ||
| ConnectionType connType = currentConnectionType(); | ||
| TableRecommendation rec = recommendationForIsolatedTable(connType); | ||
|
|
||
| jdbi.useHandle(handle -> tuner.apply(handle, rec)); | ||
|
|
||
| String built = tuner.buildAlterStatement(rec); | ||
| assertTrue(built.contains(ISOLATED_TABLE), "ALTER target table mismatch: " + built); | ||
|
|
||
| // Apply twice — second invocation must complete without throwing. | ||
| jdbi.useHandle(handle -> tuner.apply(handle, rec)); | ||
|
Comment on lines
+121
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Quality: applyExecutesAndIsIdempotent no longer verifies DB stateThe old test used This would restore the end-to-end confidence that Suggested fix: Was this helpful? React with 👍 / 👎 | Reply |
||
| } | ||
|
|
||
|
Comment on lines
+120
to
+129
|
||
| @Test | ||
| void analyzeOneRunsOnIsolatedTable() { | ||
| AutoTuner tuner = currentTuner(); | ||
| Jdbi jdbi = TestSuiteBootstrap.getJdbi(); | ||
|
|
||
| jdbi.useHandle(handle -> tuner.analyzeOne(handle, ISOLATED_TABLE)); | ||
| } | ||
|
|
||
| @Test | ||
| void dryRunDoesNotMutateReloptions() { | ||
| AutoTuner tuner = currentTuner(); | ||
| Jdbi jdbi = TestSuiteBootstrap.getJdbi(); | ||
|
|
||
| Map<String, String> before = currentSettingsFor(tuner, jdbi, READ_ONLY_PROBE_TABLE); | ||
|
|
||
| DbTuneResult result = jdbi.withHandle(tuner::analyze); | ||
| assertNotNull(result); | ||
|
|
||
| Map<String, String> after = currentSettingsFor(tuner, jdbi, READ_ONLY_PROBE_TABLE); | ||
| assertEquals(before, after, "Analyze (dry-run) must not change table settings"); | ||
| } | ||
|
|
||
| // ---- helpers ---- | ||
|
|
||
| private AutoTuner currentTuner() { | ||
| return currentConnectionType() == ConnectionType.POSTGRES | ||
| ? new PostgresAutoTuner() | ||
| : new MysqlAutoTuner(); | ||
| } | ||
|
|
||
| private ConnectionType currentConnectionType() { | ||
| return "mysql".equalsIgnoreCase(System.getProperty("databaseType", "postgres")) | ||
| ? ConnectionType.MYSQL | ||
| : ConnectionType.POSTGRES; | ||
| } | ||
|
|
||
| /** | ||
| * Builds a {@link TableRecommendation} pointing at {@link #ISOLATED_TABLE} with engine-appropriate | ||
| * settings. We construct it directly rather than going through {@code analyze()} because the | ||
| * isolated table is intentionally NOT in the static catalog — that's how we keep the apply path | ||
| * off shared production tables. | ||
| */ | ||
| private TableRecommendation recommendationForIsolatedTable(final ConnectionType connType) { | ||
| Map<String, String> recommended = | ||
| connType == ConnectionType.POSTGRES | ||
| ? Map.of("autovacuum_vacuum_scale_factor", "0.05") | ||
| : Map.of("STATS_PERSISTENT", "1", "STATS_AUTO_RECALC", "1"); | ||
| return new TableRecommendation( | ||
| ISOLATED_TABLE, Action.APPLY, 0L, 0L, Map.of(), recommended, "Isolated IT test table"); | ||
| } | ||
|
|
||
| /** | ||
| * Re-runs analyze and projects out the {@link TableRecommendation#currentSettings()} for the | ||
| * named table. Going through the same code path that built the original recommendation keeps the | ||
| * assertion stable across either dialect's parsing rules. | ||
| */ | ||
| private Map<String, String> currentSettingsFor( | ||
| final AutoTuner tuner, final Jdbi jdbi, final String tableName) { | ||
| return jdbi.withHandle(tuner::analyze).tableRecommendations().stream() | ||
| .filter(r -> tableName.equals(r.tableName())) | ||
| .findFirst() | ||
| .map(TableRecommendation::currentSettings) | ||
| .orElse(Map.of()); | ||
| } | ||
|
|
||
| private static String quoteIdent(final ConnectionType connType, final String identifier) { | ||
| if (!identifier.matches("[a-zA-Z_][a-zA-Z0-9_]*")) { | ||
| throw new IllegalArgumentException("Refusing unsafe identifier: " + identifier); | ||
| } | ||
| return connType == ConnectionType.POSTGRES ? "\"" + identifier + "\"" : "`" + identifier + "`"; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /* | ||
| * Copyright 2026 Collate | ||
| * Licensed 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.openmetadata.service.util.dbtune; | ||
|
|
||
| public enum Action { | ||
| APPLY, | ||
| TIGHTEN, | ||
| RELAX, | ||
| OK, | ||
| SKIP; | ||
|
|
||
| public boolean isActionable() { | ||
| return this == APPLY || this == TIGHTEN || this == RELAX; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in a719d47: added @execution(ExecutionMode.SAME_THREAD) on the class so the tests don't race on shared reloptions when the suite runs in parallel.