Fixes 30008: Improve server startup and write-path latency#30009
Fixes 30008: Improve server startup and write-path latency#30009harshach wants to merge 15 commits into
Conversation
github-actions
Bot
added
backend
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
✅ TypeScript Types Auto-UpdatedThe generated TypeScript types have been automatically updated based on JSON schema changes in this PR. |
Jest test CoverageUI tests summary
|
Code Review ✅ Approved 7 resolved / 7 findingsImproves server startup and write-path performance through fingerprint-gated seeding, parallelized index initialization, and shared classpath scanning. Resolved all identified seed drift and framework qualification issues during the review session. ✅ 7 resolved✅ Bug: CommonUtil.getResources now returns an immutable list
✅ Performance: Search-template gate never stamps fingerprint if any mapping is missing
✅ Quality: ClasspathScanIndex keeps ScanResult open for JVM lifetime
✅ Edge Case: Seed-data gate skips reseeding on DB drift when files are unchanged
✅ Edge Case: Warm-start seed gate reseeds every boot when email config is unset
...and 2 more resolved from earlier reviews OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
…s-startup-latency # Conflicts: # bootstrap/sql/migrations/native/2.0.0/mysql/schemaChanges.sql # bootstrap/sql/migrations/native/2.0.0/postgres/schemaChanges.sql # openmetadata-service/src/main/java/org/openmetadata/service/events/lifecycle/EntityLifecycleEventDispatcher.java # openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java # openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java # openmetadata-spec/src/main/resources/json/schema/settings/settings.json # openmetadata-ui/src/main/resources/ui/src/generated/settings/settings.ts
✅ TypeScript Types Auto-UpdatedThe generated TypeScript types have been automatically updated based on JSON schema changes in this PR. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private static final Set<SeedTable> REQUIRED_SEED_TABLES = | ||
| EnumSet.complementOf( | ||
| EnumSet.of( | ||
| SeedTable.DATA_INSIGHT_CUSTOM_CHART, SeedTable.GLOSSARY, SeedTable.GLOSSARY_TERM)); |
There was a problem hiding this comment.
shouldSeed(). When the stored checksum still matches but a restore or direct cleanup removes bundled glossary rows, this set can still mark the database clean, skip the loader, and leave the default glossary data missing across warm boots.
| private int createMissingIndexesInParallel(int parallelism) { | ||
| List<Callable<Boolean>> tasks = | ||
| entityIndexMap.entrySet().stream() | ||
| .<Callable<Boolean>>map(entry -> () -> createMissingIndex(entry)) | ||
| .toList(); | ||
| try (ExecutorService executor = | ||
| Executors.newFixedThreadPool( | ||
| parallelism, Thread.ofPlatform().name("search-init-", 0).factory())) { | ||
| int created = 0; | ||
| for (Future<Boolean> result : executor.invokeAll(tasks)) { | ||
| try { | ||
| if (result.get()) { | ||
| created++; | ||
| } | ||
| } catch (ExecutionException exception) { |
There was a problem hiding this comment.
💡 Edge Case: Parallel search-index init assumes thread-safe client calls
createMissingIndexesInParallel runs createMissingIndex (indexExists/createIndex/reconcileAliases) across up to STARTUP_SEARCH_INIT_PARALLELISM (default 8) threads simultaneously. This is correct only if the underlying ElasticSearch/OpenSearch client and the shared entityIndexMap accesses are thread-safe; any shared mutable state in reconcileAliases/createIndex could race during startup. Since results are only aggregated via Future.get(), a silent inconsistency (rather than a failure) would be hard to detect. Confirm the search client and alias reconciliation are safe under concurrency, or serialize the alias-reconcile step.
Was this helpful? React with 👍 / 👎
…s-startup-latency Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review 👍 Approved with suggestions 9 resolved / 10 findingsImproves server startup and write-path performance via seed fingerprinting and optimized classpath scanning. Ensure that the parallel search-index initialization logic correctly accounts for potential thread-safety limitations in client calls. 💡 Edge Case: Parallel search-index init assumes thread-safe client calls📄 openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java:634-648 createMissingIndexesInParallel runs createMissingIndex (indexExists/createIndex/reconcileAliases) across up to STARTUP_SEARCH_INIT_PARALLELISM (default 8) threads simultaneously. This is correct only if the underlying ElasticSearch/OpenSearch client and the shared entityIndexMap accesses are thread-safe; any shared mutable state in reconcileAliases/createIndex could race during startup. Since results are only aggregated via Future.get(), a silent inconsistency (rather than a failure) would be hard to detect. Confirm the search client and alias reconciliation are safe under concurrency, or serialize the alias-reconcile step. ✅ 9 resolved✅ Bug: CommonUtil.getResources now returns an immutable list
✅ Performance: Search-template gate never stamps fingerprint if any mapping is missing
✅ Quality: ClasspathScanIndex keeps ScanResult open for JVM lifetime
✅ Edge Case: Seed-data gate skips reseeding on DB drift when files are unchanged
✅ Edge Case: Warm-start seed gate reseeds every boot when email config is unset
...and 4 more resolved from earlier reviews 🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Describe your changes:
Fixes #30008
I improved server startup, request/write latency, and lineage query performance because warm boots repeated bundled-data and search reconciliation, writes repeated JSON serialization, and the pipeline-summary service type was interpolated into SQL.
Type of change:
High-level design:
2.0.0-SNAPSHOT, so no production migration is required, but development databases that already seeded flat control FQNs should be resetTests:
Use cases covered
Unit tests
CommonUtilTest,StartupTimerTest,SeedDataGateTest,FrameworkSeedLoaderTest,ClasspathScanIndexTest,CollectionRegistryTest,RBACConditionEvaluatorCacheTest,IndexTemplateManagerTest,EntityRepositoryStorageJsonReuseTest,EntityLifecycleEventDispatcherTest,RequestLatencyContextTest,MicrometerBundleTest,SettingsRowMapperTest, andJsonUtilsTestSeedDataGateTestandFrameworkSeedLoaderTestpassed 9/9 combined;IndexTemplateManagerTestpassed 10/10Backend integration tests
openmetadata-integration-tests/for new/changed backend behavior.PipelineResourceIT.java,LineageResourceIT.java,SeedDataPresenceIT.java,AIFrameworkControlResourceIT.java, andIndexTemplateIT.javaIngestion integration tests
Playwright (UI) tests
Manual testing performed
mvn spotless:apply,mvn spotless:check, schema validation, andgit diff --checkUI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Greptile Summary
This PR improves startup checks, search setup, and write-path performance. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Comments Outside Diff (1)
openmetadata-service/src/main/java/org/openmetadata/service/seeding/SeedDataGate.java, line 3187-3191 (link)json/data/*seed files, and many seed loaders now return before their per-row insert checks whenshouldSeed()is false. IfSTARTUP_CHECKSUMSsurvives a restore while seeded rows such as bots, docstore rows, task form schemas, test definitions, tags, workflow definitions, or notification templates are missing, this check can still mark the database clean and skip the loaders. Those defaults remain absent until an operator forces seeding.Reviews (14): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile
Context used (3)