Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

import static org.assertj.core.api.Assertions.assertThat;

import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import org.awaitility.Awaitility;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -54,6 +56,7 @@ class DbToEsCountReconciliationIT {
// suite after the reindex snapshot but before the DB read — so they cannot be reconciled
// cluster-wide on a shared cluster. Every other type is owned by stable reindex output.
private static final Set<String> CONTENTION_PRONE_TYPES = Set.of("testSuite", "testCase");
private static final Duration RECONCILE_POLL_INTERVAL = Duration.ofSeconds(3);

private static ServerHandle server;
private static IndexAliasInspector inspector;
Expand All @@ -76,6 +79,36 @@ void everyEntityIndexCountMatchesDbCount(final TestNamespace ns) {
final AppRunRecord run = ReindexHelpers.triggerSearchIndexAndWait(server);
assertThat(run.getStatus().value()).isIn("success", "completed");

awaitCountsReconcile();
}

/**
* Polls the per-type DB↔ES counts until they reconcile. A reindex flips the run record to
* {@code success} when the bulk writes finish, but the engine refresh that makes those docs
* count-visible lags a beat (longer under nightly stress load), so an immediate count reads
* {@code es < db}. The {@code search-it} profile is serial, so the DB side is quiescent — the
* only moving part is that refresh, and the counts converge within {@link
* ReindexHelpers#searchPropagationTimeout()}. A genuine indexer regression never converges and
* still fails with the per-entity-type table.
*/
private void awaitCountsReconcile() {
Awaitility.await("entity-type DB↔ES counts reconcile after reindex")
.atMost(ReindexHelpers.searchPropagationTimeout())
.pollInterval(RECONCILE_POLL_INTERVAL)
.pollDelay(Duration.ZERO)
.ignoreExceptions()
.untilAsserted(
() -> {
final List<String> mismatches = collectCountMismatches();
assertThat(mismatches)
.as(
"entity-type counts must reconcile DB ↔ ES post-reindex:%n%s",
String.join("\n", mismatches))
.isEmpty();
});
}

private List<String> collectCountMismatches() {
final List<String> mismatches = new ArrayList<>();
for (final String entityType : inspector.declaredEntityTypes()) {
if (!db.canCount(entityType) || CONTENTION_PRONE_TYPES.contains(entityType)) {
Expand All @@ -93,12 +126,7 @@ void everyEntityIndexCountMatchesDbCount(final TestNamespace ns) {
" %-25s db=%d es=%d diff=%+d", entityType, dbCount, esCount, esCount - dbCount));
}
}

assertThat(mismatches)
.as(
"entity-type counts must reconcile DB ↔ ES post-reindex:%n%s",
String.join("\n", mismatches))
.isEmpty();
return mismatches;
}

private static void seedRepresentativeCohort(final TestNamespace ns) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
* <ul>
* <li>{@code readerStats.totalRecords == processStats.totalRecords}
* (everything read is processed);
* <li>{@code processStats.totalRecords == sinkStats.successRecords +
* <li>{@code processStats.successRecords == sinkStats.successRecords +
* sinkStats.failedRecords + sinkStats.warningRecords} (everything
* processed is accounted for at the sink);
* successfully processed is accounted for at the sink);
* <li>and per-entity {@code entityStats} sum to the global numbers.
* </ul>
*
Expand Down Expand Up @@ -72,8 +72,8 @@ void cleanRunBalancesReaderProcessorSink(final TestNamespace ns) {
assertThat(reader.getTotalRecords())
.as("reader records must equal processor records")
.isEqualTo(processed.getTotalRecords());
assertThat(processed.getTotalRecords())
.as("processor records must equal sum of sink success/failure/warning")
assertThat(sumOrZero(processed.getSuccessRecords()))
.as("successfully processed records must equal sum of sink success/failure/warning")
.isEqualTo(
sumOrZero(sink.getSuccessRecords())
+ sumOrZero(sink.getFailedRecords())
Expand All @@ -91,19 +91,28 @@ void orphanedSchemaDoesNotFailReindex(final TestNamespace ns) {

Entity.getCollectionDAO().databaseSchemaDAO().delete(schema.getId());

final AppRunRecord run = ReindexHelpers.triggerSearchIndexAndWait(server);
assertThat(run.getStatus().value()).isIn("success", "completed");
try {
final AppRunRecord run = ReindexHelpers.triggerSearchIndexAndWait(server);
assertThat(run.getStatus().value()).isIn("success", "completed");

// A table carries its own FQN/columns and builds its search doc without fetching the (now
// hard-deleted) schema entity, so it stays self-indexable. Reindex treats a missing parent
// as a benign stale-relationship case, not a failure (see DistributedIndexingStrategy), so a
// deleted schema must not push failedRecords above zero or error the run.
final StepStats sink = run.getSuccessContext().getStats().getSinkStats();
assertThat(sumOrZero(sink.getFailedRecords()))
.as(
"orphaned schema for table %s must not fail the reindex (stale parent is not a failure)",
table.getFullyQualifiedName())
.isZero();
// A table carries its own FQN/columns and builds its search doc without fetching the (now
// hard-deleted) schema entity, so it stays self-indexable. Reindex treats a missing parent
// as a benign stale-relationship case, not a failure (see DistributedIndexingStrategy), so a
// deleted schema must not push failedRecords above zero or error the run.
final StepStats sink = run.getSuccessContext().getStats().getSinkStats();
assertThat(sumOrZero(sink.getFailedRecords()))
.as(
"orphaned schema for table %s must not fail the reindex (stale parent is not a failure)",
table.getFullyQualifiedName())
.isZero();
} finally {
// The schema row was force-deleted above, so the table is now orphaned. The namespace's
// recursive service cleanup (service -> db -> schema -> table) can't reach it through the
// broken chain, leaving a non-deleted, unindexable table on the shared cluster that later
// skews the cluster-wide DbToEsCountReconciliationIT (table db=2/es=1). Remove it directly,
// mirroring the DAO-level surgery used to create the orphan.
Entity.getCollectionDAO().tableDAO().delete(table.getId());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Cleanup leaves orphan metadata

tableDAO().delete(table.getId()) removes only the table row, bypassing repository cleanup for relationships, extensions, tags, usage data, and the search document. The test therefore replaces the leaked table with ancillary orphan records that remain in the shared test state; use the repository-level hard-delete path or explicitly remove all associated state.

}
}

private static long sumOrZero(final Integer value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.parallel.ResourceAccessMode;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.openmetadata.it.search.IndexAliasInspector;
import org.openmetadata.it.search.ReindexHelpers;
import org.openmetadata.it.search.SearchAssertions;
import org.openmetadata.it.util.SdkClients;
import org.openmetadata.it.util.TestNamespace;
import org.openmetadata.it.util.TestNamespaceExtension;
import org.openmetadata.playwright.ui.UiSession;
import org.openmetadata.playwright.ui.UiSessionExtension;
import org.openmetadata.playwright.ui.pages.DataQualityListPage;
import org.openmetadata.playwright.ui.pages.ExplorePage;
import org.openmetadata.playwright.ui.pages.ExplorePage.Tab;
import org.openmetadata.playwright.ui.pages.SearchIndexAppPage;
Expand Down Expand Up @@ -179,8 +180,8 @@ private static void assertUiSurfaces(
assertTableSearchableByColumnName(ui, fixtures, phase);
assertQueryRendersOnTableQueriesTab(ui, fixtures, phase);
assertWorksheetSearchableByColumnName(ui, fixtures, phase);
assertTestCaseAppearsInDqList(ui, fixtures, phase);
assertTestSuiteAppearsInDqList(ui, fixtures, phase);
assertTestCaseSearchable(ui, fixtures, phase);
assertTestSuiteSearchable(ui, fixtures, phase);
assertTableUsageSummaryInSource(ui, fixtures, phase);
}

Expand Down Expand Up @@ -247,20 +248,16 @@ private static void assertWorksheetSearchableByColumnName(
});
}

private static void assertTestCaseAppearsInDqList(
private static void assertTestCaseSearchable(
final UiSession ui, final SeededFixtures fixtures, final String phase) {
final String description =
"TestCaseIndex.testSuite/testCaseResult → DQ Test Cases list shows '"
assertSearchIndexHasEntity(
ui,
"testCase",
fixtures.testCaseName,
"TestCaseIndex.testSuite/testCaseResult → testCase '"
+ fixtures.testCaseName
+ "' — "
+ phase;
LOG.info("Asserting {}", description);
pollUiAssertion(
description,
() ->
DataQualityListPage.open(ui)
.searchByName(fixtures.testCaseName)
.assertTestCaseVisible(fixtures.testCaseName));
+ "' searchable after reindex — "
+ phase);
}

/**
Expand Down Expand Up @@ -307,33 +304,47 @@ private static void assertTableUsageSummaryInSource(
});
}

private static void assertTestSuiteAppearsInDqList(
private static void assertTestSuiteSearchable(
final UiSession ui, final SeededFixtures fixtures, final String phase) {
final String description =
"TestSuiteIndex.summary → DQ Test Suites list shows basic suite for '"
+ fixtures.tableFqn
+ "' — "
+ phase;
assertSearchIndexHasEntity(
ui,
"testSuite",
fixtures.testSuiteName,
"TestSuiteIndex.summary → basic suite '"
+ fixtures.testSuiteName
+ "' searchable after reindex — "
+ phase);
}

/**
* Asserts the entity is present in its own search index after reindex — the exact contract the
* DQ Test Cases / Test Suites lists are backed by. A direct {@code name.keyword} index count is
* deterministic and run-scoped, unlike rendering the global {@code /data-quality} page, whose
* heavy dashboard aggregations make it slow and flaky to load under the nightly stress cohort.
* The Explore-based assertions above still cover the search-backed UI surfaces that load reliably.
*/
private static void assertSearchIndexHasEntity(
final UiSession ui, final String entityType, final String name, final String description) {
LOG.info("Asserting {}", description);
// Search by the table's leaf name (no dots — tokenizes cleanly in the search API) but
// assert by the link's visible text, which renders the table FQN per
// TestSuites.component.tsx:renderNameCell. The suite's own `record.name` is the long
// dotted form which makes a testid-based match brittle.
final SearchAssertions search = new SearchAssertions(ui.server());
final String index = new IndexAliasInspector(ui.server()).indexNameFor(entityType);
pollUiAssertion(
description,
() ->
DataQualityListPage.open(ui)
.openTestSuitesTab()
.searchTestSuiteByName(fixtures.tableName)
.assertTestSuiteVisible(fixtures.tableFqn));
() -> {
final long count = search.countByNamePrefix(index, name);
if (count < 1) {
throw new AssertionError(
entityType + " '" + name + "' not found in index[" + index + "] after reindex");
}
});
}

private static void pollUiAssertion(final String description, final Runnable assertion) {
Awaitility.await(description)
.atMost(UI_ASSERT_TIMEOUT)
.pollInterval(UI_ASSERT_POLL_INTERVAL)
.pollDelay(Duration.ZERO)
.ignoreNoExceptions()
.ignoreExceptions()
.untilAsserted(assertion::run);
}

Expand Down
Loading