This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git
The following commit(s) were added to refs/heads/master by this push:
new 082c75814 Stop stale controller pipelines from writing cluster views
after a session change (#3178)
082c75814 is described below
commit 082c75814d2fbaca6b9a69d4e9b124091cbe4be1
Author: Junkai Xue <[email protected]>
AuthorDate: Fri Nov 6 23:53:38 2026 -0800
Stop stale controller pipelines from writing cluster views after a session
change (#3178)
TestConsecutiveZkSessionExpiry.testDistributedController keeps failing in
CI with the cluster fully converged but the external view stuck at the
state computed right after the demoted controller lost its session. Every
failing run also shows a DEFAULT/TASK pipeline on the demoted controller
that started under its old leader session, blocked across both reconnects
and was only stopped seconds later by MessageDispatchStage's session
check, i.e. the stages before it ran on stale data under the wrong
session. The leader never re-reads the external view after its first
refresh, so a stray write is never corrected.
- Pipeline.handle re-checks before every stage that the event was created
in the controller's current session and stops the pipeline with a
StageException otherwise, as MessageDispatchStage already does for
message sending but for every stage.
- AbstractAsyncBaseStage performs the same check when the queued task
actually runs, so the external view, target external view and persisted
assignment writers skip stale events.
- The check (AbstractBaseStage.isEventSessionStale) does not wait for a
disconnected manager: while disconnected the session is unknown and
processing continues as before.
- The test's failure dump now prints each znode's version and modified
time so a future failure shows whether the external view was written
before or after the last current state change.
---
.../pipeline/AbstractAsyncBaseStage.java | 9 ++
.../controller/pipeline/AbstractBaseStage.java | 52 ++++++
.../apache/helix/controller/pipeline/Pipeline.java | 10 ++
.../pipeline/TestPipelineStaleSession.java | 175 +++++++++++++++++++++
.../controller/stages/TestAsyncBaseStage.java | 83 ++++++++++
.../manager/TestConsecutiveZkSessionExpiry.java | 26 ++-
6 files changed, 349 insertions(+), 6 deletions(-)
diff --git
a/helix-core/src/main/java/org/apache/helix/controller/pipeline/AbstractAsyncBaseStage.java
b/helix-core/src/main/java/org/apache/helix/controller/pipeline/AbstractAsyncBaseStage.java
index 18c278b7b..72c941b21 100644
---
a/helix-core/src/main/java/org/apache/helix/controller/pipeline/AbstractAsyncBaseStage.java
+++
b/helix-core/src/main/java/org/apache/helix/controller/pipeline/AbstractAsyncBaseStage.java
@@ -42,6 +42,15 @@ public abstract class AbstractAsyncBaseStage extends
AbstractBaseStage {
long startTimestamp = System.currentTimeMillis();
logger.info("START AsyncProcess: {}", taskType);
try {
+ // The task runs some time after the pipeline queued it. If the
controller's session has
+ // changed in between (session expiry, possibly with another
controller now leading), the
+ // event's data is stale and must not be written to the cluster.
+ if (isEventSessionStale(event)) {
+ logger.warn("Skip AsyncProcess: {} for event {} of cluster {}
because its session {} is "
+ + "not the controller's current session anymore", taskType,
event.getEventId(),
+ event.getClusterName(), describeEventSession(event));
+ return;
+ }
execute(event);
} catch (InterruptedException e) {
logger.warn("Process {} has been interrupted", taskType, e);
diff --git
a/helix-core/src/main/java/org/apache/helix/controller/pipeline/AbstractBaseStage.java
b/helix-core/src/main/java/org/apache/helix/controller/pipeline/AbstractBaseStage.java
index 8e0e37f8d..6b5d20cdd 100644
---
a/helix-core/src/main/java/org/apache/helix/controller/pipeline/AbstractBaseStage.java
+++
b/helix-core/src/main/java/org/apache/helix/controller/pipeline/AbstractBaseStage.java
@@ -20,15 +20,22 @@ package org.apache.helix.controller.pipeline;
*/
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
+import org.apache.helix.HelixException;
+import org.apache.helix.HelixManager;
import org.apache.helix.common.DedupEventProcessor;
import org.apache.helix.controller.stages.AttributeName;
import org.apache.helix.controller.stages.ClusterEvent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class AbstractBaseStage implements Stage {
+ private static final Logger LOG =
LoggerFactory.getLogger(AbstractBaseStage.class);
+
protected String _eventId;
@Override
@@ -64,6 +71,51 @@ public class AbstractBaseStage implements Stage {
return className;
}
+ /**
+ * Tells whether the controller's ZooKeeper session has changed since the
event was created, in
+ * which case the event's data is stale: the session expired while the event
was queued or being
+ * processed, and another controller may already lead the cluster. Stages
must not act on such an
+ * event, in particular not write cluster-wide state (messages, views,
persisted assignments).
+ * <p>
+ * Every event the controller creates in production carries the leader
session it was created in
+ * ({@link AttributeName#EVENT_SESSION}); events without that attribute
(only built by tests) are
+ * never stale. The check does not wait for a disconnected manager to
reconnect: while the manager
+ * is disconnected the session is unknown (it may still be the same one
after a reconnect), so the
+ * event is not reported as stale and processing goes on as before, relying
on the ZkClient retry
+ * logic and on {@code MessageDispatchStage}'s blocking session check that
guards message sending.
+ * The manager's session id is updated when it handles the new session, so
an event is recognized
+ * as stale from that point on; that is when the stale pipelines observed in
practice resume, as
+ * they were blocked on ZooKeeper reads during the expiry.
+ * @param event the event being processed
+ * @return true if the event's manager is connected and its session differs
from the event's
+ * session
+ */
+ static boolean isEventSessionStale(ClusterEvent event) {
+ Optional<String> eventSession =
event.getAttribute(AttributeName.EVENT_SESSION.name());
+ HelixManager manager =
event.getAttribute(AttributeName.helixmanager.name());
+ if (eventSession == null || !eventSession.isPresent() || manager == null) {
+ // No session recorded: either a test-built event, or the controller was
not leader when
+ // the event was created. GenericHelixController#handleEvent already
filters the latter.
+ return false;
+ }
+ try {
+ return manager.isConnected() &&
!eventSession.get().equals(manager.getSessionId());
+ } catch (HelixException e) {
+ // The manager got disconnected in between; the session is unknown, not
known to be stale.
+ LOG.debug("Cannot determine the session of event {}: manager is not
connected",
+ event.getEventId(), e);
+ return false;
+ }
+ }
+
+ /**
+ * @return the session the event was created in, for log messages
+ */
+ static String describeEventSession(ClusterEvent event) {
+ Optional<String> eventSession =
event.getAttribute(AttributeName.EVENT_SESSION.name());
+ return eventSession == null ? "NOT_PRESENT" :
eventSession.orElse("NOT_PRESENT");
+ }
+
public static <T> Future asyncExecute(ExecutorService service, Callable<T>
task) {
if (service != null) {
return service.submit(task);
diff --git
a/helix-core/src/main/java/org/apache/helix/controller/pipeline/Pipeline.java
b/helix-core/src/main/java/org/apache/helix/controller/pipeline/Pipeline.java
index 4196e23b4..9fbb563a8 100644
---
a/helix-core/src/main/java/org/apache/helix/controller/pipeline/Pipeline.java
+++
b/helix-core/src/main/java/org/apache/helix/controller/pipeline/Pipeline.java
@@ -69,6 +69,16 @@ public class Pipeline {
return;
}
for (Stage stage : _stages) {
+ // A stage can block for a long time on ZooKeeper reads while the
controller's session is
+ // expiring. Re-check the session before every stage so that an event
created under a
+ // previous session does not continue into the stages that write
cluster-wide state.
+ if (AbstractBaseStage.isEventSessionStale(event)) {
+ throw new StageException(String.format(
+ "Event %s was created in session %s, which is not the controller's
current session"
+ + " anymore. Stop %s pipeline for cluster %s before stage %s",
+ event.getEventId(), AbstractBaseStage.describeEventSession(event),
_pipelineType,
+ event.getClusterName(), stage.getStageName()));
+ }
long startTime = System.currentTimeMillis();
stage.preProcess();
diff --git
a/helix-core/src/test/java/org/apache/helix/controller/pipeline/TestPipelineStaleSession.java
b/helix-core/src/test/java/org/apache/helix/controller/pipeline/TestPipelineStaleSession.java
new file mode 100644
index 000000000..307ddca8e
--- /dev/null
+++
b/helix-core/src/test/java/org/apache/helix/controller/pipeline/TestPipelineStaleSession.java
@@ -0,0 +1,175 @@
+package org.apache.helix.controller.pipeline;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.helix.HelixException;
+import org.apache.helix.HelixManager;
+import org.apache.helix.controller.stages.AttributeName;
+import org.apache.helix.controller.stages.ClusterEvent;
+import org.apache.helix.controller.stages.ClusterEventType;
+import org.mockito.Mockito;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+/**
+ * Verifies that a pipeline stops processing an event whose session is not the
controller's session
+ * anymore, and keeps processing it in all other cases.
+ */
+public class TestPipelineStaleSession {
+ private static final String CURRENT_SESSION = "session-2";
+ private static final String EXPIRED_SESSION = "session-1";
+
+ @Test
+ public void testStagesRunWhenEventSessionIsCurrent() throws Exception {
+ List<String> processed = new ArrayList<>();
+ Pipeline pipeline = pipelineWithStages(processed);
+
+ pipeline.handle(event(Optional.of(CURRENT_SESSION),
connectedManager(CURRENT_SESSION)));
+
+ Assert.assertEquals(processed, Arrays.asList("first", "second"));
+ }
+
+ @Test
+ public void testPipelineStopsWhenEventSessionIsStale() throws Exception {
+ List<String> processed = new ArrayList<>();
+ Pipeline pipeline = pipelineWithStages(processed);
+
+ try {
+ pipeline.handle(event(Optional.of(EXPIRED_SESSION),
connectedManager(CURRENT_SESSION)));
+ Assert.fail("A stale event must not be processed");
+ } catch (StageException expected) {
+ Assert.assertTrue(expected.getMessage().contains(EXPIRED_SESSION),
expected.getMessage());
+ }
+ Assert.assertTrue(processed.isEmpty(), "No stage should run for a stale
event: " + processed);
+ }
+
+ @Test
+ public void testStaleSessionIsDetectedBetweenStages() throws Exception {
+ List<String> processed = new ArrayList<>();
+ HelixManager manager = connectedManager(EXPIRED_SESSION);
+ Pipeline pipeline = new Pipeline("DEFAULT");
+ pipeline.addStage(new RecordingStage("first", processed));
+ // The session changes while this stage is running.
+ pipeline.addStage(new AbstractBaseStage() {
+ @Override
+ public void process(ClusterEvent event) {
+ processed.add("session-change");
+ Mockito.when(manager.getSessionId()).thenReturn(CURRENT_SESSION);
+ }
+ });
+ pipeline.addStage(new RecordingStage("third", processed));
+
+ try {
+ pipeline.handle(event(Optional.of(EXPIRED_SESSION), manager));
+ Assert.fail("The stage after the session change must not run");
+ } catch (StageException expected) {
+ // expected
+ }
+ Assert.assertEquals(processed, Arrays.asList("first", "session-change"));
+ }
+
+ @Test
+ public void testStagesRunWhenEventHasNoSession() throws Exception {
+ List<String> processed = new ArrayList<>();
+ Pipeline pipeline = pipelineWithStages(processed);
+
+ // Events built by tests carry no session attribute at all.
+ ClusterEvent noSession = new ClusterEvent("cluster",
ClusterEventType.OnDemandRebalance);
+ noSession.addAttribute(AttributeName.helixmanager.name(),
connectedManager(CURRENT_SESSION));
+ pipeline.handle(noSession);
+ // Events created while the controller was not the leader carry an empty
session; those are
+ // filtered by GenericHelixController before they reach a pipeline.
+ pipeline.handle(event(Optional.empty(),
connectedManager(CURRENT_SESSION)));
+ // Events without a manager cannot be checked.
+ ClusterEvent noManager = new ClusterEvent("cluster",
ClusterEventType.OnDemandRebalance);
+ noManager.addAttribute(AttributeName.EVENT_SESSION.name(),
Optional.of(EXPIRED_SESSION));
+ pipeline.handle(noManager);
+
+ Assert.assertEquals(processed,
+ Arrays.asList("first", "second", "first", "second", "first",
"second"));
+ }
+
+ @Test
+ public void testStagesRunWhileManagerIsDisconnected() throws Exception {
+ List<String> processed = new ArrayList<>();
+ Pipeline pipeline = pipelineWithStages(processed);
+
+ // While disconnected the session is unknown, so the event is not stale.
The manager may
+ // either report not connected or throw from getSessionId().
+ HelixManager disconnected = Mockito.mock(HelixManager.class);
+ Mockito.when(disconnected.isConnected()).thenReturn(false);
+ Mockito.when(disconnected.getSessionId()).thenReturn(CURRENT_SESSION);
+ pipeline.handle(event(Optional.of(EXPIRED_SESSION), disconnected));
+
+ HelixManager throwing = Mockito.mock(HelixManager.class);
+ Mockito.when(throwing.isConnected()).thenReturn(true);
+ Mockito.when(throwing.getSessionId()).thenThrow(new HelixException("not
connected"));
+ pipeline.handle(event(Optional.of(EXPIRED_SESSION), throwing));
+
+ Assert.assertEquals(processed, Arrays.asList("first", "second", "first",
"second"));
+ }
+
+ private static Pipeline pipelineWithStages(List<String> processed) {
+ Pipeline pipeline = new Pipeline("DEFAULT");
+ pipeline.addStage(new RecordingStage("first", processed));
+ pipeline.addStage(new RecordingStage("second", processed));
+ return pipeline;
+ }
+
+ private static ClusterEvent event(Optional<String> eventSession,
HelixManager manager) {
+ ClusterEvent event = new ClusterEvent("cluster",
ClusterEventType.OnDemandRebalance);
+ event.addAttribute(AttributeName.EVENT_SESSION.name(), eventSession);
+ event.addAttribute(AttributeName.helixmanager.name(), manager);
+ return event;
+ }
+
+ private static HelixManager connectedManager(String sessionId) {
+ HelixManager manager = Mockito.mock(HelixManager.class);
+ Mockito.when(manager.isConnected()).thenReturn(true);
+ Mockito.when(manager.getSessionId()).thenReturn(sessionId);
+ Mockito.when(manager.getInstanceName()).thenReturn("controller");
+ return manager;
+ }
+
+ private static class RecordingStage extends AbstractBaseStage {
+ private final String _name;
+ private final List<String> _processed;
+
+ RecordingStage(String name, List<String> processed) {
+ _name = name;
+ _processed = processed;
+ }
+
+ @Override
+ public void process(ClusterEvent event) {
+ _processed.add(_name);
+ }
+
+ @Override
+ public String getStageName() {
+ return _name;
+ }
+ }
+}
diff --git
a/helix-core/src/test/java/org/apache/helix/controller/stages/TestAsyncBaseStage.java
b/helix-core/src/test/java/org/apache/helix/controller/stages/TestAsyncBaseStage.java
index e5c0efeb4..d3372ff67 100644
---
a/helix-core/src/test/java/org/apache/helix/controller/stages/TestAsyncBaseStage.java
+++
b/helix-core/src/test/java/org/apache/helix/controller/stages/TestAsyncBaseStage.java
@@ -2,12 +2,16 @@ package org.apache.helix.controller.stages;
import java.util.HashMap;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import org.apache.helix.HelixManager;
import org.apache.helix.TestHelper;
import org.apache.helix.common.DedupEventProcessor;
import org.apache.helix.controller.pipeline.AbstractAsyncBaseStage;
import org.apache.helix.controller.pipeline.AsyncWorkerType;
+import org.mockito.Mockito;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -51,6 +55,85 @@ public class TestAsyncBaseStage {
blockingAsyncStage.reset();
}
+ @Test
+ public void testStaleSessionEventIsSkipped() throws Exception {
+ DedupEventProcessor<String, Runnable> worker =
+ new DedupEventProcessor<String, Runnable>("ClusterName",
DEFAULT_WORKER_TYPE.name()) {
+ @Override
+ protected void handleEvent(Runnable event) {
+ event.run();
+ }
+ };
+ worker.start();
+ Map<AsyncWorkerType, DedupEventProcessor<String, Runnable>>
asyncFIFOWorkerPool =
+ new HashMap<>();
+ asyncFIFOWorkerPool.put(DEFAULT_WORKER_TYPE, worker);
+
+ HelixManager manager = Mockito.mock(HelixManager.class);
+ Mockito.when(manager.isConnected()).thenReturn(true);
+ Mockito.when(manager.getSessionId()).thenReturn("session-2");
+ Mockito.when(manager.getInstanceName()).thenReturn("controller");
+
+ try {
+ // The session the event was created in has expired by the time the task
runs: skip it.
+ RecordingAsyncStage stage = new RecordingAsyncStage();
+ stage.process(event(asyncFIFOWorkerPool, manager,
Optional.of("session-1")));
+ waitForWorker(worker);
+ Assert.assertFalse(stage._executed, "A task for a stale event must not
be executed");
+
+ // The event's session is still the controller's session: execute it.
+ stage = new RecordingAsyncStage();
+ stage.process(event(asyncFIFOWorkerPool, manager,
Optional.of("session-2")));
+ waitForWorker(worker);
+ Assert.assertTrue(stage._executed, "A task for a current event must be
executed");
+
+ // While the manager is disconnected the session is unknown: keep the
previous behavior.
+ Mockito.when(manager.isConnected()).thenReturn(false);
+ stage = new RecordingAsyncStage();
+ stage.process(event(asyncFIFOWorkerPool, manager,
Optional.of("session-1")));
+ waitForWorker(worker);
+ Assert.assertTrue(stage._executed,
+ "A task must be executed while the session cannot be determined");
+ } finally {
+ worker.shutdown();
+ }
+ }
+
+ private static ClusterEvent event(
+ Map<AsyncWorkerType, DedupEventProcessor<String, Runnable>>
asyncFIFOWorkerPool,
+ HelixManager manager, Optional<String> eventSession) {
+ ClusterEvent event = new ClusterEvent("ClusterName",
ClusterEventType.OnDemandRebalance);
+ event.addAttribute(AttributeName.AsyncFIFOWorkerPool.name(),
asyncFIFOWorkerPool);
+ event.addAttribute(AttributeName.helixmanager.name(), manager);
+ event.addAttribute(AttributeName.EVENT_SESSION.name(), eventSession);
+ return event;
+ }
+
+ /**
+ * Queues a marker task behind whatever the worker is processing and waits
for it, so that every
+ * task queued before it has been handled (or skipped) once this returns.
+ */
+ private static void waitForWorker(DedupEventProcessor<String, Runnable>
worker)
+ throws InterruptedException {
+ CountDownLatch drained = new CountDownLatch(1);
+ worker.queueEvent("marker", drained::countDown);
+ Assert.assertTrue(drained.await(5, TimeUnit.SECONDS), "Worker did not
drain its queue");
+ }
+
+ private class RecordingAsyncStage extends AbstractAsyncBaseStage {
+ private volatile boolean _executed = false;
+
+ @Override
+ public AsyncWorkerType getAsyncWorkerType() {
+ return DEFAULT_WORKER_TYPE;
+ }
+
+ @Override
+ public void execute(ClusterEvent event) {
+ _executed = true;
+ }
+ }
+
private class BlockingAsyncStage extends AbstractAsyncBaseStage {
public boolean _isFinished = false;
public boolean _isStarted = false;
diff --git
a/helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java
b/helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java
index 57469a671..5739d2012 100644
---
a/helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java
+++
b/helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java
@@ -26,6 +26,7 @@ import java.util.concurrent.CountDownLatch;
import org.apache.helix.HelixDataAccessor;
import org.apache.helix.HelixManager;
+import org.apache.helix.HelixProperty;
import org.apache.helix.PreConnectCallback;
import org.apache.helix.PropertyKey;
import org.apache.helix.TestHelper;
@@ -361,14 +362,15 @@ public class TestConsecutiveZkSessionExpiry extends
ZkUnitTestBase {
LiveInstance leader =
accessor.getProperty(keyBuilder.controllerLeader());
sb.append("LEADER: ").append(leader == null ? "none"
- : leader.getInstanceName() + " (session " +
leader.getEphemeralOwner() + ")")
- .append('\n');
+ : leader.getInstanceName() + " (session " +
leader.getEphemeralOwner() + ", "
+ + describeStat(leader) + ")").append('\n');
Map<String, LiveInstance> liveInstances =
accessor.getChildValuesMap(keyBuilder.liveInstances(), false);
for (Map.Entry<String, LiveInstance> entry : liveInstances.entrySet()) {
sb.append("LIVEINSTANCE ").append(entry.getKey()).append(": session ")
- .append(entry.getValue().getEphemeralOwner()).append('\n');
+ .append(entry.getValue().getEphemeralOwner()).append(", ")
+ .append(describeStat(entry.getValue())).append('\n');
}
for (String instance : accessor.getChildNames(keyBuilder.instances())) {
@@ -377,8 +379,9 @@ public class TestConsecutiveZkSessionExpiry extends
ZkUnitTestBase {
accessor.getChildValues(keyBuilder.currentStates(instance,
session), false);
for (CurrentState currentState : currentStates) {
sb.append("CURRENTSTATE
").append(instance).append('/').append(session).append('/')
- .append(currentState.getResourceName()).append(": ")
- .append(currentState.getPartitionStateMap()).append('\n');
+ .append(currentState.getResourceName()).append(" (session
field ")
+ .append(currentState.getSessionId()).append(",
").append(describeStat(currentState))
+ .append("):
").append(currentState.getPartitionStateMap()).append('\n');
}
}
List<Message> messages =
accessor.getChildValues(keyBuilder.messages(instance), false);
@@ -395,7 +398,8 @@ public class TestConsecutiveZkSessionExpiry extends
ZkUnitTestBase {
List<ExternalView> externalViews =
accessor.getChildValues(keyBuilder.externalViews(), false);
for (ExternalView externalView : externalViews) {
- sb.append("EXTERNALVIEW
").append(externalView.getResourceName()).append(": ")
+ sb.append("EXTERNALVIEW
").append(externalView.getResourceName()).append(" (")
+ .append(describeStat(externalView)).append("): ")
.append(externalView.getRecord().getMapFields()).append('\n');
}
} catch (Exception e) {
@@ -403,4 +407,14 @@ public class TestConsecutiveZkSessionExpiry extends
ZkUnitTestBase {
}
System.out.println(sb);
}
+
+ /**
+ * Version and modification time of a znode, so that the order in which the
dumped znodes were
+ * last written can be reconstructed (e.g. whether the external view was
written before or after
+ * the last current state change).
+ */
+ private static String describeStat(HelixProperty property) {
+ HelixProperty.Stat stat = property.getStat();
+ return "version " + stat.getVersion() + ", modified " + new
Date(stat.getModifiedTime());
+ }
}