Skip to content

Commit b8f9def

Browse files
glebashnikglebashnik
authored
Remove restart for internal redeployment when local LLM config is unchanged (#36389)
1 parent d53c2d3 commit b8f9def

2 files changed

Lines changed: 126 additions & 37 deletions

File tree

‎config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForLocalLLMValidator.java‎

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,31 @@
11
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
22
package com.yahoo.vespa.model.application.validation.change;
33

4+
import ai.vespa.llm.clients.LlmLocalClientConfig;
45
import com.yahoo.config.provision.ClusterSpec;
56
import com.yahoo.text.Text;
7+
import com.yahoo.vespa.config.ConfigPayload;
68
import com.yahoo.vespa.model.AbstractService;
79
import com.yahoo.vespa.model.VespaModel;
810
import com.yahoo.vespa.model.application.validation.Validation.ChangeContext;
911
import com.yahoo.vespa.model.container.ApplicationContainerCluster;
12+
import com.yahoo.vespa.model.container.component.Component;
1013

1114
import java.util.HashSet;
15+
import java.util.Map;
1216
import java.util.Set;
1317
import java.util.logging.Logger;
1418

1519
import static java.util.logging.Level.INFO;
20+
import static java.util.stream.Collectors.toUnmodifiableMap;
1621
import static java.util.stream.Collectors.toUnmodifiableSet;
1722

1823
/**
1924
* If using local LLMs, this validator will make sure that restartOnDeploy is set for
2025
* configs for this cluster.
2126
*
22-
* @author lesters
27+
* @author Lester Solbakken
28+
* @author glebashnik
2329
*/
2430
public class RestartOnDeployForLocalLLMValidator implements ChangeValidator {
2531

@@ -29,26 +35,73 @@ public class RestartOnDeployForLocalLLMValidator implements ChangeValidator {
2935

3036
@Override
3137
public void validate(ChangeContext context) {
32-
var previousClustersWithLocalLLM = findClustersWithLocalLLMs(context.previousModel());
33-
var nextClustersWithLocalLLM = findClustersWithLocalLLMs(context.model());
38+
Set<ClusterSpec.Id> previousClustersWithLocalLLM = findClustersWithLocalLLMs(context.previousModel());
39+
Set<ClusterSpec.Id> nextClustersWithLocalLLM = findClustersWithLocalLLMs(context.model());
40+
41+
// For clusters with local LLM in both generations: always restart on external redeploy,
42+
// but only restart on internal redeploy if the local LLM config changed.
43+
for (ClusterSpec.Id clusterId : intersect(previousClustersWithLocalLLM, nextClustersWithLocalLLM)) {
44+
ApplicationContainerCluster previousCluster = context.previousModel().getContainerClusters().get(
45+
clusterId.value());
46+
ApplicationContainerCluster nextCluster = context.model().getContainerClusters().get(clusterId.value());
47+
boolean llmConfigChanged = localLLMConfigChanged(
48+
context.previousModel(), previousCluster,
49+
context.model(), nextCluster
50+
);
3451

35-
// Only restart services if we use a local LLM in both the next and previous generation
36-
for (var clusterId : intersect(previousClustersWithLocalLLM, nextClustersWithLocalLLM)) {
3752
String message = Text.format("Need to restart services in %s due to use of local LLM", clusterId);
3853
context.require(new VespaRestartAction(
3954
clusterId,
4055
message,
41-
context.model().getContainerClusters().get(clusterId.value()).getContainers()
42-
.stream().map(AbstractService::getServiceInfo).toList(),
43-
VespaRestartAction.ConfigChange.DEFER_UNTIL_RESTART));
56+
nextCluster.getContainers().stream().map(AbstractService::getServiceInfo).toList(),
57+
!llmConfigChanged,
58+
VespaRestartAction.ConfigChange.DEFER_UNTIL_RESTART
59+
));
4460
log.log(INFO, message);
4561
}
4662
}
4763

64+
private boolean localLLMConfigChanged(VespaModel previousModel, ApplicationContainerCluster previousCluster,
65+
VespaModel nextModel, ApplicationContainerCluster nextCluster) {
66+
Map<String, Component<?, ?>> previousLLMs = findLocalLLMComponents(previousCluster);
67+
Map<String, Component<?, ?>> nextLLMs = findLocalLLMComponents(nextCluster);
68+
69+
if (!previousLLMs.keySet().equals(nextLLMs.keySet())) {
70+
return true;
71+
}
72+
73+
for (String id : nextLLMs.keySet()) {
74+
LlmLocalClientConfig previousConfig = previousModel.getConfig(
75+
LlmLocalClientConfig.class, previousLLMs.get(id).getConfigId());
76+
LlmLocalClientConfig nextConfig = nextModel.getConfig(
77+
LlmLocalClientConfig.class, nextLLMs.get(id).getConfigId());
78+
79+
if (configChanged(previousConfig, nextConfig)) {
80+
return true;
81+
}
82+
}
83+
return false;
84+
}
85+
86+
/**
87+
* Compares configs via their serialized payload. ConfigPayload serializes ModelNode using
88+
* ModelNode.getValue(), which returns the configured reference string rather than the resolved
89+
* Path — correctly detecting model path changes even before model references are resolved.
90+
*/
91+
private boolean configChanged(LlmLocalClientConfig previous, LlmLocalClientConfig next) {
92+
return !ConfigPayload.fromInstance(previous).toString(true)
93+
.equals(ConfigPayload.fromInstance(next).toString(true));
94+
}
95+
96+
private Map<String, Component<?, ?>> findLocalLLMComponents(ApplicationContainerCluster cluster) {
97+
return cluster.getAllComponents().stream()
98+
.filter(c -> c.getClassId().getName().equals(LOCAL_LLM_COMPONENT))
99+
.collect(toUnmodifiableMap(c -> c.getComponentId().stringValue(), c -> c));
100+
}
101+
48102
private Set<ClusterSpec.Id> findClustersWithLocalLLMs(VespaModel model) {
49103
return model.getContainerClusters().values().stream()
50-
.filter(cluster -> cluster.getAllComponents().stream()
51-
.anyMatch(component -> component.getClassId().getName().equals(LOCAL_LLM_COMPONENT)))
104+
.filter(cluster -> !findLocalLLMComponents(cluster).isEmpty())
52105
.map(ApplicationContainerCluster::id)
53106
.collect(toUnmodifiableSet());
54107
}

‎config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForLocalLLMValidatorTest.java‎

Lines changed: 63 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import com.yahoo.config.model.api.ConfigChangeAction;
55
import com.yahoo.config.model.deploy.DeployState;
66
import com.yahoo.config.model.deploy.TestDeployState;
7-
import com.yahoo.config.model.deploy.TestProperties;
87
import com.yahoo.text.Text;
98
import com.yahoo.vespa.model.VespaModel;
109
import com.yahoo.vespa.model.application.validation.ValidationTester;
@@ -14,10 +13,12 @@
1413
import java.util.List;
1514

1615
import static org.junit.jupiter.api.Assertions.assertEquals;
16+
import static org.junit.jupiter.api.Assertions.assertFalse;
1717
import static org.junit.jupiter.api.Assertions.assertTrue;
1818

1919
/**
20-
* @author lesters
20+
* @author Lester Solbakken
21+
* @author glebashnik
2122
*/
2223
public class RestartOnDeployForLocalLLMValidatorTest {
2324

@@ -26,38 +27,68 @@ public class RestartOnDeployForLocalLLMValidatorTest {
2627
@Test
2728
void validate_no_restart_on_deploy() {
2829
VespaModel current = createModel();
29-
VespaModel next = createModel(withComponent(LOCAL_LLM_COMPONENT));
30+
VespaModel next = createModel(withLocalLLMComponent("models/model.gguf", 1));
3031
List<ConfigChangeAction> result = validateModel(current, next);
3132
assertEquals(0, result.size());
3233
}
3334

3435
@Test
35-
void validate_restart_on_deploy() {
36-
VespaModel current = createModel(withComponent(LOCAL_LLM_COMPONENT));
37-
VespaModel next = createModel(withComponent(LOCAL_LLM_COMPONENT));
36+
void validate_restart_on_deploy_llm_config_unchanged() {
37+
VespaModel current = createModel(withLocalLLMComponent("models/model.gguf", 1));
38+
VespaModel next = createModel(withLocalLLMComponent("models/model.gguf", 1));
3839
List<ConfigChangeAction> result = validateModel(current, next);
3940
assertEquals(1, result.size());
4041
assertTrue(result.get(0).validationId().isEmpty());
41-
assertEquals("Need to restart services in cluster 'cluster1' due to use of local LLM", result.get(0).getMessage());
42+
assertEquals(
43+
"Need to restart services in cluster 'cluster1' due to use of local LLM", result.get(0).getMessage());
44+
assertTrue(
45+
result.get(0).ignoreForInternalRedeploy(),
46+
"Restart must be ignored for internal redeployment when LLM config is unchanged"
47+
);
48+
}
49+
50+
@Test
51+
void validate_restart_on_deploy_llm_config_changed() {
52+
VespaModel current = createModel(withLocalLLMComponent("models/model.gguf", 1));
53+
VespaModel next = createModel(withLocalLLMComponent("models/model.gguf", 2));
54+
List<ConfigChangeAction> result = validateModel(current, next);
55+
assertEquals(1, result.size());
56+
assertFalse(
57+
result.get(0).ignoreForInternalRedeploy(),
58+
"Restart must not be ignored for internal redeployment when LLM config changed"
59+
);
60+
}
61+
62+
@Test
63+
void validate_restart_on_deploy_model_path_changed() {
64+
VespaModel current = createModel(withLocalLLMComponent("models/model-a.gguf", 1));
65+
VespaModel next = createModel(withLocalLLMComponent("models/model-b.gguf", 1));
66+
List<ConfigChangeAction> result = validateModel(current, next);
67+
assertEquals(1, result.size());
68+
assertFalse(result.get(0).ignoreForInternalRedeploy(), "Restart must not be ignored when model path changed");
4269
}
4370

4471
private static List<ConfigChangeAction> validateModel(VespaModel current, VespaModel next) {
45-
return ValidationTester.validateChanges(new RestartOnDeployForLocalLLMValidator(),
46-
next,
47-
deployStateBuilder().previousModel(current).build());
72+
return ValidationTester.validateChanges(
73+
new RestartOnDeployForLocalLLMValidator(),
74+
next,
75+
deployStateBuilder().previousModel(current).build()
76+
);
4877
}
4978

5079
private static VespaModel createModel(String component) {
51-
var xml = Text.format("""
52-
<services version='1.0'>
53-
<container id='cluster1' version='1.0'>
54-
<http>
55-
<server id='server1' port='8080'/>
56-
</http>
57-
%s
58-
</container>
59-
</services>
60-
""", component);
80+
var xml = Text.format(
81+
"""
82+
<services version='1.0'>
83+
<container id='cluster1' version='1.0'>
84+
<http>
85+
<server id='server1' port='8080'/>
86+
</http>
87+
%s
88+
</container>
89+
</services>
90+
""", component
91+
);
6192
DeployState.Builder builder = deployStateBuilder();
6293
return new VespaModelCreatorWithMockPkg(null, xml).create(builder);
6394
}
@@ -66,16 +97,21 @@ private static VespaModel createModel() {
6697
return createModel("");
6798
}
6899

69-
private static String withComponent(String componentClass) {
70-
return Text.format("<component id='llm' class='%s' />", componentClass);
100+
private static String withLocalLLMComponent(String modelPath, int parallelRequests) {
101+
return Text.format(
102+
"""
103+
<component id='llm' class='%s'>
104+
<config name="ai.vespa.llm.clients.llm-local-client">
105+
<model path="%s"/>
106+
<parallelRequests>%d</parallelRequests>
107+
</config>
108+
</component>""", RestartOnDeployForLocalLLMValidatorTest.LOCAL_LLM_COMPONENT, modelPath,
109+
parallelRequests
110+
);
71111
}
72112

73113
private static DeployState.Builder deployStateBuilder() {
74114
return TestDeployState.createBuilder();
75115
}
76116

77-
private static void assertStartsWith(String expected, List<ConfigChangeAction> result) {
78-
assertTrue(result.get(0).getMessage().startsWith(expected));
79-
}
80-
81-
}
117+
}

0 commit comments

Comments
 (0)
⚡