fix: provisioning race condition and noisy ClickHouse logs
Defer provisionAsync() until after the transaction commits using TransactionSynchronization.afterCommit(). Previously the @Async thread raced the @Transactional commit — findById returned null because the tenant INSERT wasn't visible yet. Downgrade ClickHouse UNKNOWN_TABLE errors to DEBUG level in InfrastructureService. These are expected on fresh installs before any cameleer-server has created the tables. Make the onboarding slug field read-only (derived from org name). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -224,9 +224,13 @@ public class InfrastructureService {
|
|||||||
.put(table, cnt);
|
.put(table, cnt);
|
||||||
}
|
}
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
|
if (e.getMessage() != null && e.getMessage().contains("UNKNOWN_TABLE")) {
|
||||||
|
log.debug("ClickHouse table '{}' does not exist yet — skipping", table);
|
||||||
|
} else {
|
||||||
log.error("Failed to query ClickHouse table '{}' for tenant stats: {}", table, e.getMessage(), e);
|
log.error("Failed to query ClickHouse table '{}' for tenant stats: {}", table, e.getMessage(), e);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
log.error("Failed to get ClickHouse tenant stats: {}", e.getMessage(), e);
|
log.error("Failed to get ClickHouse tenant stats: {}", e.getMessage(), e);
|
||||||
throw new RuntimeException("Failed to get ClickHouse tenant stats", e);
|
throw new RuntimeException("Failed to get ClickHouse tenant stats", e);
|
||||||
@@ -256,10 +260,14 @@ public class InfrastructureService {
|
|||||||
result.add(new ChTableStats(table, rs.getLong("cnt")));
|
result.add(new ChTableStats(table, rs.getLong("cnt")));
|
||||||
}
|
}
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
|
if (e.getMessage() != null && e.getMessage().contains("UNKNOWN_TABLE")) {
|
||||||
|
log.debug("ClickHouse table '{}' does not exist yet — skipping", table);
|
||||||
|
} else {
|
||||||
log.error("Failed to query ClickHouse table '{}' for tenant '{}': {}",
|
log.error("Failed to query ClickHouse table '{}' for tenant '{}': {}",
|
||||||
table, tenantId, e.getMessage(), e);
|
table, tenantId, e.getMessage(), e);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
log.error("Failed to get ClickHouse tenant detail for '{}': {}", tenantId, e.getMessage(), e);
|
log.error("Failed to get ClickHouse tenant detail for '{}': {}", tenantId, e.getMessage(), e);
|
||||||
throw new RuntimeException("Failed to get ClickHouse tenant detail for: " + tenantId, e);
|
throw new RuntimeException("Failed to get ClickHouse tenant detail for: " + tenantId, e);
|
||||||
|
|||||||
@@ -26,6 +26,8 @@ import org.springframework.context.annotation.Lazy;
|
|||||||
import org.springframework.scheduling.annotation.Async;
|
import org.springframework.scheduling.annotation.Async;
|
||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
import org.springframework.transaction.annotation.Transactional;
|
import org.springframework.transaction.annotation.Transactional;
|
||||||
|
import org.springframework.transaction.support.TransactionSynchronization;
|
||||||
|
import org.springframework.transaction.support.TransactionSynchronizationManager;
|
||||||
|
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
@@ -116,9 +118,19 @@ public class VendorTenantService {
|
|||||||
AuditAction.TENANT_CREATE, "provision:" + tenant.getSlug(),
|
AuditAction.TENANT_CREATE, "provision:" + tenant.getSlug(),
|
||||||
null, null, "SUCCESS", null);
|
null, null, "SUCCESS", null);
|
||||||
|
|
||||||
// 4. Provision server asynchronously (Docker containers, health check, config push)
|
// 4. Provision server asynchronously AFTER transaction commits
|
||||||
|
// (the async thread needs the tenant row to be visible)
|
||||||
if (tenantProvisioner.isAvailable()) {
|
if (tenantProvisioner.isAvailable()) {
|
||||||
self.provisionAsync(tenant.getId(), tenant.getSlug(), tenant.getTier().name(), license.getToken(), actorId);
|
UUID tenantId = tenant.getId();
|
||||||
|
String slug = tenant.getSlug();
|
||||||
|
String tierName = tenant.getTier().name();
|
||||||
|
String token = license.getToken();
|
||||||
|
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
|
||||||
|
@Override
|
||||||
|
public void afterCommit() {
|
||||||
|
self.provisionAsync(tenantId, slug, tierName, token, actorId);
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
return tenant;
|
return tenant;
|
||||||
|
|||||||
@@ -17,11 +17,14 @@ import net.siegeln.cameleer.saas.tenant.TenantService;
|
|||||||
import net.siegeln.cameleer.saas.tenant.TenantStatus;
|
import net.siegeln.cameleer.saas.tenant.TenantStatus;
|
||||||
import net.siegeln.cameleer.saas.tenant.Tier;
|
import net.siegeln.cameleer.saas.tenant.Tier;
|
||||||
import net.siegeln.cameleer.saas.tenant.dto.CreateTenantRequest;
|
import net.siegeln.cameleer.saas.tenant.dto.CreateTenantRequest;
|
||||||
|
import org.junit.jupiter.api.AfterEach;
|
||||||
import org.junit.jupiter.api.BeforeEach;
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.extension.ExtendWith;
|
import org.junit.jupiter.api.extension.ExtendWith;
|
||||||
import org.mockito.Mock;
|
import org.mockito.Mock;
|
||||||
import org.mockito.junit.jupiter.MockitoExtension;
|
import org.mockito.junit.jupiter.MockitoExtension;
|
||||||
|
import org.springframework.transaction.support.TransactionSynchronization;
|
||||||
|
import org.springframework.transaction.support.TransactionSynchronizationManager;
|
||||||
|
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
import java.time.Instant;
|
import java.time.Instant;
|
||||||
@@ -89,6 +92,28 @@ class VendorTenantServiceTest {
|
|||||||
tenantService, tenantRepository, licenseService,
|
tenantService, tenantRepository, licenseService,
|
||||||
tenantProvisioner, serverApiClient, logtoClient, logtoConfig,
|
tenantProvisioner, serverApiClient, logtoClient, logtoConfig,
|
||||||
auditService, provisioningProps, dataCleanupService, tenantDatabaseService, vendorTenantService);
|
auditService, provisioningProps, dataCleanupService, tenantDatabaseService, vendorTenantService);
|
||||||
|
|
||||||
|
// Enable transaction synchronization so afterCommit callbacks can be registered
|
||||||
|
if (!TransactionSynchronizationManager.isSynchronizationActive()) {
|
||||||
|
TransactionSynchronizationManager.initSynchronization();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@AfterEach
|
||||||
|
void tearDown() {
|
||||||
|
if (TransactionSynchronizationManager.isSynchronizationActive()) {
|
||||||
|
TransactionSynchronizationManager.clearSynchronization();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Simulate transaction commit — runs all registered afterCommit callbacks. */
|
||||||
|
private void flushAfterCommit() {
|
||||||
|
var syncs = TransactionSynchronizationManager.getSynchronizations();
|
||||||
|
for (TransactionSynchronization sync : syncs) {
|
||||||
|
sync.afterCommit();
|
||||||
|
}
|
||||||
|
TransactionSynchronizationManager.clearSynchronization();
|
||||||
|
TransactionSynchronizationManager.initSynchronization();
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- Helpers ---
|
// --- Helpers ---
|
||||||
@@ -155,8 +180,9 @@ class VendorTenantServiceTest {
|
|||||||
when(tenantRepository.save(any(TenantEntity.class))).thenAnswer(inv -> inv.getArgument(0));
|
when(tenantRepository.save(any(TenantEntity.class))).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
vendorTenantService.createAndProvision(request, actorId);
|
vendorTenantService.createAndProvision(request, actorId);
|
||||||
|
flushAfterCommit();
|
||||||
|
|
||||||
// provisionAsync modifies the tenant entity in-place (runs synchronously in unit tests)
|
// provisionAsync runs via afterCommit callback (synchronously in unit tests)
|
||||||
assertThat(tenant.getStatus()).isEqualTo(TenantStatus.ACTIVE);
|
assertThat(tenant.getStatus()).isEqualTo(TenantStatus.ACTIVE);
|
||||||
assertThat(tenant.getServerEndpoint()).isEqualTo("http://server:8080");
|
assertThat(tenant.getServerEndpoint()).isEqualTo("http://server:8080");
|
||||||
assertThat(tenant.getProvisionError()).isNull();
|
assertThat(tenant.getProvisionError()).isNull();
|
||||||
@@ -178,8 +204,9 @@ class VendorTenantServiceTest {
|
|||||||
when(tenantRepository.save(any(TenantEntity.class))).thenAnswer(inv -> inv.getArgument(0));
|
when(tenantRepository.save(any(TenantEntity.class))).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
vendorTenantService.createAndProvision(request, actorId);
|
vendorTenantService.createAndProvision(request, actorId);
|
||||||
|
flushAfterCommit();
|
||||||
|
|
||||||
// provisionAsync modifies the tenant entity in-place (runs synchronously in unit tests)
|
// provisionAsync runs via afterCommit callback (synchronously in unit tests)
|
||||||
assertThat(tenant.getProvisionError()).isEqualTo("Docker failure");
|
assertThat(tenant.getProvisionError()).isEqualTo("Docker failure");
|
||||||
assertThat(tenant.getStatus()).isEqualTo(TenantStatus.PROVISIONING);
|
assertThat(tenant.getStatus()).isEqualTo(TenantStatus.PROVISIONING);
|
||||||
verify(tenantRepository, times(2)).save(tenant);
|
verify(tenantRepository, times(2)).save(tenant);
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { useState, useEffect } from 'react';
|
import { useState } from 'react';
|
||||||
import { Card, Input, Button, FormField, Alert } from '@cameleer/design-system';
|
import { Card, Input, Button, FormField, Alert } from '@cameleer/design-system';
|
||||||
import cameleerLogo from '@cameleer/design-system/assets/cameleer-logo.svg';
|
import cameleerLogo from '@cameleer/design-system/assets/cameleer-logo.svg';
|
||||||
import { api } from '../api/client';
|
import { api } from '../api/client';
|
||||||
@@ -14,16 +14,10 @@ interface TenantResponse {
|
|||||||
|
|
||||||
export function OnboardingPage() {
|
export function OnboardingPage() {
|
||||||
const [name, setName] = useState('');
|
const [name, setName] = useState('');
|
||||||
const [slug, setSlug] = useState('');
|
|
||||||
const [slugTouched, setSlugTouched] = useState(false);
|
|
||||||
const [loading, setLoading] = useState(false);
|
const [loading, setLoading] = useState(false);
|
||||||
const [error, setError] = useState<string | null>(null);
|
const [error, setError] = useState<string | null>(null);
|
||||||
|
|
||||||
useEffect(() => {
|
const slug = toSlug(name);
|
||||||
if (!slugTouched) {
|
|
||||||
setSlug(toSlug(name));
|
|
||||||
}
|
|
||||||
}, [name, slugTouched]);
|
|
||||||
|
|
||||||
async function handleSubmit(e: React.FormEvent) {
|
async function handleSubmit(e: React.FormEvent) {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
@@ -78,10 +72,8 @@ export function OnboardingPage() {
|
|||||||
<Input
|
<Input
|
||||||
id="onboard-slug"
|
id="onboard-slug"
|
||||||
value={slug}
|
value={slug}
|
||||||
onChange={(e) => { setSlugTouched(true); setSlug(e.target.value); }}
|
|
||||||
placeholder="acme-corp"
|
placeholder="acme-corp"
|
||||||
disabled={loading}
|
readOnly
|
||||||
required
|
|
||||||
/>
|
/>
|
||||||
</FormField>
|
</FormField>
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user