Fixes 30008: Improve server startup and write-path latency#30009
Fixes 30008: Improve server startup and write-path latency#30009harshach wants to merge 9 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 checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
✅ 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 4 resolved / 4 findingsImproves server startup and write-path performance through checksum-based seed gating, classpath caching, and optimized JSON serialization. All identified findings regarding seed-data and search-template reconciliation logic have been resolved. ✅ 4 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
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (!force | ||
| && buildResult.failures() == 0 | ||
| && !SeedDataGate.getInstance() | ||
| .shouldUpdateSearchTemplates(fingerprint, createdIndexCount)) { | ||
| LOG.info("Index templates are unchanged; skipping {} template updates", templates.size()); | ||
| SeedDataGate.getInstance().recordSearchTemplateFingerprint(fingerprint); | ||
| return; |
There was a problem hiding this comment.
createdIndexCount == 0. On a warm boot where all indexes exist but the search cluster templates were deleted or restored stale, the local checksum still matches the database row, so startup records the same fingerprint again and leaves the cluster templates missing or stale. Later indexing can then run with missing or incorrect mappings until a forced path or mapping change happens.
| private String getEmailTemplateProvider() { | ||
| Settings emailSettings = | ||
| systemRepository.getConfigWithKey(SettingsType.EMAIL_CONFIGURATION.toString()); | ||
| if (emailSettings == null || emailSettings.getConfigValue() == null) { | ||
| return null; | ||
| } | ||
| SmtpSettings smtpSettings = | ||
| JsonUtils.convertValue(emailSettings.getConfigValue(), SmtpSettings.class); | ||
| return smtpSettings.getTemplates() == null | ||
| ? OPENMETADATA_EMAIL_TEMPLATE_PROVIDER | ||
| : smtpSettings.getTemplates().value(); | ||
| } |
There was a problem hiding this comment.
⚠️ Edge Case: Warm-start seed gate reseeds every boot when email config is unset
When no EMAIL_CONFIGURATION settings row exists, SeedDataGate.getEmailTemplateProvider() returns null (SeedDataGate.java:316-317), and RequiredSeedRows.selectEmailDocuments(null) then requires BOTH openmetadata and collate email documents to be present (RequiredSeedRows.java:58-60). However the actual seeder DocumentRepository.getEntitiesFromSeedData() loads only ONE provider's templates — defaulting to openmetadata (DocumentRepository.java:64-75) via SettingsCache which returns the OPENMETADATA default rather than null. As a result the collate email documents are never seeded, hasRequiredSeedRows always returns false, and the drift check reseeds on every warm boot — defeating the startup optimization this PR introduces. Make getEmailTemplateProvider default to OPENMETADATA when the config row is absent (mirroring the seeder / DefaultOperationalConfigProvider), instead of returning null.
Default to the OPENMETADATA provider when no email config exists, matching the seeder which only loads one provider's documents.:
private String getEmailTemplateProvider() {
Settings emailSettings =
systemRepository.getConfigWithKey(SettingsType.EMAIL_CONFIGURATION.toString());
if (emailSettings == null || emailSettings.getConfigValue() == null) {
return OPENMETADATA_EMAIL_TEMPLATE_PROVIDER;
}
SmtpSettings smtpSettings =
JsonUtils.convertValue(emailSettings.getConfigValue(), SmtpSettings.class);
return smtpSettings.getTemplates() == null
? OPENMETADATA_EMAIL_TEMPLATE_PROVIDER
: smtpSettings.getTemplates().value();
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| @@ -88,7 +91,7 @@ private static void seedBundle( | |||
| EntityReference frameworkRef = saved.getEntityReference(); | |||
| for (JsonNode controlNode : controlsNode) { | |||
There was a problem hiding this comment.
💡 Bug: Flat control FQN can skip seeding controls sharing a name across frameworks
FrameworkSeedLoader now stores each AIFrameworkControl with a flat FQN of just the control name (FrameworkSeedLoader.java:94,100) instead of framework.control, and dedups via findByNameOrNull on that flat name. If two frameworks define controls with the same name, the second control resolves to the first framework's already-seeded control and is silently skipped (continue at line 96-97), so it is never created. This matches the flat identity the drift check expects, but silently drops legitimately distinct controls when names collide across frameworks. Consider qualifying the control FQN with the framework (and updating the drift-check identity accordingly) if cross-framework control-name collisions are possible.
Was this helpful? React with 👍 / 👎
Code Review
|
| Compact |
|
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:
Tests:
Use cases covered
Unit tests
CommonUtilTest,StartupTimerTest,SeedDataGateTest,ClasspathScanIndexTest,CollectionRegistryTest,RBACConditionEvaluatorCacheTest,IndexTemplateManagerTest,EntityRepositoryStorageJsonReuseTest,EntityLifecycleEventDispatcherTest,RequestLatencyContextTest,MicrometerBundleTest,SettingsRowMapperTest, andJsonUtilsTestSeedDataGateTestpassed 8/8 andIndexTemplateManagerTestpassed 10/10Backend integration tests
openmetadata-integration-tests/for new/changed backend behavior.PipelineResourceIT.java,LineageResourceIT.java,SeedDataPresenceIT.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 and write-path performance across the backend. 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 (8): Last reviewed commit: "fix: validate warm-start state against l..." | Re-trigger Greptile
Context used (3)