-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
feat: support incremental configuration synchronization client #5288
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
Changes from 12 commits
4e8a699
26a0e1b
2dd9352
b13968c
24bda1d
cf5145f
8cafda2
72676c0
065a092
e69f098
a6a412a
07bb41c
a4b356c
5e37f55
8512776
5a76da9
a4a2d05
04f50ac
1b20e4e
4f06613
26e8db5
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package com.ctrip.framework.apollo.configservice.controller; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.biz.config.BizConfig; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.biz.entity.Release; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.common.entity.AppNamespace; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.common.utils.WebUtils; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -26,10 +27,13 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.core.ConfigConsts; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.core.dto.ApolloConfig; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.core.dto.ApolloNotificationMessages; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.core.dto.ConfigurationChange; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.core.enums.ConfigSyncType; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.ctrip.framework.apollo.tracer.Tracer; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.common.base.Strings; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.common.collect.Lists; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.common.collect.Maps; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.common.collect.Sets; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.gson.Gson; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.gson.reflect.TypeToken; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.GetMapping; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -42,6 +46,9 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import javax.servlet.http.HttpServletResponse; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.lang.reflect.Type; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Arrays; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.LinkedHashSet; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Objects; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -59,6 +66,8 @@ public class ConfigController { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final NamespaceUtil namespaceUtil; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final InstanceConfigAuditUtil instanceConfigAuditUtil; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final Gson gson; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final BizConfig bizConfig; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static final Type configurationTypeReference = new TypeToken<Map<String, String>>() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }.getType(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -68,12 +77,14 @@ public ConfigController( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final AppNamespaceServiceWithCache appNamespaceService, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final NamespaceUtil namespaceUtil, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final InstanceConfigAuditUtil instanceConfigAuditUtil, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final Gson gson) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final Gson gson, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final BizConfig bizConfig) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.configService = configService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.appNamespaceService = appNamespaceService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.namespaceUtil = namespaceUtil; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.instanceConfigAuditUtil = instanceConfigAuditUtil; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.gson = gson; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.bizConfig=bizConfig; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @GetMapping(value = "/{appId}/{clusterName}/{namespace:.+}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -132,10 +143,10 @@ public ApolloConfig queryConfig(@PathVariable String appId, @PathVariable String | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auditReleases(appId, clusterName, dataCenter, clientIp, releases); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String mergedReleaseKey = releases.stream().map(Release::getReleaseKey) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String latestMergedReleaseKey = releases.stream().map(Release::getReleaseKey) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .collect(Collectors.joining(ConfigConsts.CLUSTER_NAMESPACE_SEPARATOR)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (mergedReleaseKey.equals(clientSideReleaseKey)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (latestMergedReleaseKey.equals(clientSideReleaseKey)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Client side configuration is the same with server side, return 304 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Tracer.logEvent("Apollo.Config.NotModified", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -144,8 +155,42 @@ public ApolloConfig queryConfig(@PathVariable String appId, @PathVariable String | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ApolloConfig apolloConfig = new ApolloConfig(appId, appClusterNameLoaded, originalNamespace, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mergedReleaseKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apolloConfig.setConfigurations(mergeReleaseConfigurations(releases)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| latestMergedReleaseKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<String, String> latestConfigurations=mergeReleaseConfigurations(releases); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if(bizConfig.isConfigServiceChangeCacheEnabled()){ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LinkedHashSet<String> clientSideReleaseKeys = Sets.newLinkedHashSet( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Arrays.stream(clientSideReleaseKey.split("\\+")).collect(Collectors.toList())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<String, Release> historyReleases = configService.findReleasesByReleaseKeys( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clientSideReleaseKeys); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //find history releases | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (historyReleases != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //order by clientSideReleaseKeys | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<Release> historyReleasesWithOrder = new ArrayList<>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (String item : clientSideReleaseKeys) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Release release = historyReleases.get(item); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if(release!=null){ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| historyReleasesWithOrder.add(release); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<String, String> historyConfigurations = mergeReleaseConfigurations | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (historyReleasesWithOrder); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<ConfigurationChange> configurationChanges = configService.calcConfigurationChanges | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (latestConfigurations, historyConfigurations); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apolloConfig.setConfigurationChanges(configurationChanges); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apolloConfig.setConfigSyncType(ConfigSyncType.INCREMENTALSYNC.getValue()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jackie-coming marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return apolloConfig; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+179
to
+188
Contributor
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. Add error handling for configuration change calculation The configuration change calculation lacks error handling for potential failures. Add try-catch block: + try {
Map<String, String> historyConfigurations = mergeReleaseConfigurations(historyReleasesWithOrder);
List<ConfigurationChange> configurationChanges = configService.calcConfigurationChanges(
latestConfigurations, historyConfigurations);
apolloConfig.setConfigurationChanges(configurationChanges);
apolloConfig.setConfigSyncType(ConfigSyncType.INCREMENTALSYNC.getValue());
+ } catch (Exception e) {
+ Tracer.logError("Failed to calculate configuration changes", e);
+ // Fallback to full sync
+ apolloConfig.setConfigurations(latestConfigurations);
+ apolloConfig.setConfigSyncType(ConfigSyncType.FULLSYNC.getValue());
+ }📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+166
to
+191
Contributor
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. Handle empty or null Currently, the code checks if Suggested solution: Modify the condition to check for empty if (historyReleases != null && !historyReleases.isEmpty()) {
// Existing logic
} else {
// Fallback to full synchronization or handle the situation appropriately
apolloConfig.setConfigurations(latestConfigurations);
} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apolloConfig.setConfigurations(latestConfigurations); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Tracer.logEvent("Apollo.Config.Found", assembleKey(appId, appClusterNameLoaded, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| originalNamespace, dataCenter)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,9 +21,17 @@ | |
| import com.ctrip.framework.apollo.core.ConfigConsts; | ||
| import com.ctrip.framework.apollo.core.dto.ApolloNotificationMessages; | ||
|
|
||
| import com.ctrip.framework.apollo.core.dto.ConfigurationChange; | ||
| import com.ctrip.framework.apollo.core.enums.ConfigurationChangeType; | ||
| import com.google.common.base.Strings; | ||
|
|
||
| import com.google.common.collect.Lists; | ||
| import com.google.common.collect.Sets; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Set; | ||
|
|
||
| /** | ||
| * @author Jason Song([email protected]) | ||
|
|
@@ -93,6 +101,52 @@ private Release findRelease(String clientAppId, String clientIp, String clientLa | |
| return release; | ||
| } | ||
|
|
||
| public List<ConfigurationChange> calcConfigurationChanges( | ||
| Map<String, String> latestReleaseConfigurations, Map<String, String> historyConfigurations) { | ||
| if (latestReleaseConfigurations == null) { | ||
| latestReleaseConfigurations = new HashMap<>(); | ||
| } | ||
|
|
||
| if (historyConfigurations == null) { | ||
| historyConfigurations = new HashMap<>(); | ||
| } | ||
|
|
||
| Set<String> previousKeys = historyConfigurations.keySet(); | ||
| Set<String> currentKeys = latestReleaseConfigurations.keySet(); | ||
|
|
||
| Set<String> commonKeys = Sets.intersection(previousKeys, currentKeys); | ||
| Set<String> newKeys = Sets.difference(currentKeys, commonKeys); | ||
| Set<String> removedKeys = Sets.difference(previousKeys, commonKeys); | ||
|
|
||
| List<ConfigurationChange> changes = Lists.newArrayList(); | ||
|
|
||
| for (String newKey : newKeys) { | ||
| changes.add(new ConfigurationChange(newKey, latestReleaseConfigurations.get(newKey), | ||
| ConfigurationChangeType.ADDED)); | ||
| } | ||
|
|
||
| for (String removedKey : removedKeys) { | ||
| changes.add(new ConfigurationChange(removedKey, null, ConfigurationChangeType.DELETED)); | ||
| } | ||
|
|
||
| for (String commonKey : commonKeys) { | ||
| String previousValue = historyConfigurations.get(commonKey); | ||
| String currentValue = latestReleaseConfigurations.get(commonKey); | ||
| if (com.google.common.base.Objects.equal(previousValue, currentValue)) { | ||
| continue; | ||
| } | ||
| changes.add( | ||
| new ConfigurationChange(commonKey, currentValue, ConfigurationChangeType.MODIFIED)); | ||
| } | ||
|
|
||
| return changes; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, Release> findReleasesByReleaseKeys(Set<String> releaseKeys){ | ||
| return null; | ||
| } | ||
|
Comment on lines
+145
to
+147
Contributor
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. Implement or document the null return behavior. The current implementation returns
Example improvement: @Override
public Map<String, Release> findReleasesByReleaseKeys(Set<String> releaseKeys) {
// Option 1: Implement the method
throw new UnsupportedOperationException("Not implemented in abstract class");
// Option 2: If null is intentional, document why
/**
* {@inheritDoc}
* @return null as this base implementation doesn't support release key lookup
*/
return null;
// Option 3: Make it abstract
// public abstract Map<String, Release> findReleasesByReleaseKeys(Set<String> releaseKeys);
} |
||
|
|
||
| /** | ||
| * Find active release by id | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,7 @@ | |
| /** | ||
| * @author Jason Song([email protected]) | ||
| */ | ||
| public interface ConfigService extends ReleaseMessageListener { | ||
| public interface ConfigService extends ReleaseMessageListener, IncrementalSyncConfigService { | ||
|
|
||
| /** | ||
| * Load config | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.