Skip to content
This repository has been archived by the owner on May 18, 2021. It is now read-only.

Fix bugs with allow-duplicate-installations-on-an-agent. #58

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix bugs with allow-duplicate-installations-on-an-agent. #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39ec3cd
Fix bugs with allow-duplicate-installations-on-an-agent.
Jun 25, 2014
f849162
code review feedback
Oct 27, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions airship-coordinator/src/main/java/io/airlift/airship/coordinator/AdminResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ public class AdminResource
{
private final Coordinator coordinator;
private final Repository repository;
private final boolean allowDuplicateInstallationsOnAnAgent;

@Inject
public AdminResource(Coordinator coordinator, Repository repository)
public AdminResource(Coordinator coordinator, Repository repository, CoordinatorConfig config)
{
this.coordinator = coordinator;
this.repository = repository;
this.allowDuplicateInstallationsOnAnAgent = config.isAllowDuplicateInstallationsOnAnAgent();
}

@GET
Expand Down Expand Up @@ -80,7 +82,7 @@ public Response getAllAgents(@Context UriInfo uriInfo)
Predicate<AgentStatus> agentPredicate = AgentFilterBuilder.build(uriInfo,
transform(coordinator.getAgents(), idGetter()),
transform(allSlotStatus, SlotStatus.uuidGetter()),
false,
allowDuplicateInstallationsOnAnAgent,
repository);

List<AgentStatus> agents = coordinator.getAgents(agentPredicate);
Expand Down
41 changes: 34 additions & 7 deletions airship-coordinator/src/main/java/io/airlift/airship/coordinator/Coordinator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import javax.annotation.PostConstruct;

import java.net.URI;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -65,6 +66,7 @@
import java.util.concurrent.TimeUnit;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.Iterables.concat;
import static com.google.common.collect.Iterables.filter;
import static com.google.common.collect.Iterables.transform;
Expand Down Expand Up @@ -438,19 +440,44 @@ public List<SlotStatus> install(Predicate<AgentStatus> filter, int limit, Assign
{
final Installation installation = InstallationUtils.toInstallation(repository, assignment);

List<RemoteAgent> targetAgents = new ArrayList<>(selectAgents(filter, installation));
targetAgents = targetAgents.subList(0, Math.min(targetAgents.size(), limit));
List<RemoteAgent> candidateAgents = new ArrayList<>(selectAgents(filter, installation));
// Prune agent list if more than required exist.
if (candidateAgents.size() > limit) {
candidateAgents = candidateAgents.subList(0, limit);
}

int totalInstalls = limit;
int installsPerHost = allowDuplicateInstallationsOnAnAgent ? limit / candidateAgents.size() : 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this only works if limit divides evenly into the nodes. For example if you have a target of 10 and 3 nodes you will only get nine installations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I Presto when doing stuff like this, we use:

candidateAgents = Iterables.limit(Iterables.cycle(candidateAgents), limit);

Once you have that, I think you can simply pass it to the existing logic.


ImmutableList.Builder<Map.Entry<RemoteAgent, Integer>> builder = ImmutableList.builder();
for (RemoteAgent agent : candidateAgents) {
int installsToAssign = Math.min(totalInstalls, installsPerHost);
if (installsToAssign <= 0) {
break;
}
builder.add(new AbstractMap.SimpleImmutableEntry<>(agent, installsToAssign));
totalInstalls -= installsToAssign;
}

List<Map.Entry<RemoteAgent, Integer>> targetAgents = builder.build();

return parallel(targetAgents, new Function<RemoteAgent, SlotStatus>()
List<List<SlotStatus>> results = parallel(targetAgents, new Function<Map.Entry<RemoteAgent, Integer>, List<SlotStatus>>()
{
@Override
public SlotStatus apply(RemoteAgent agent)
public List<SlotStatus> apply(Map.Entry<RemoteAgent, Integer> entry)
{
SlotStatus slotStatus = agent.install(installation);
stateManager.setExpectedState(new ExpectedSlotStatus(slotStatus.getId(), STOPPED, installation.getAssignment()));
return slotStatus;
RemoteAgent agent = entry.getKey();
ImmutableList.Builder<SlotStatus> builder = ImmutableList.builder();
for (int i = 0; i < entry.getValue(); i++) {
SlotStatus slotStatus = agent.install(installation);
builder.add(slotStatus);
stateManager.setExpectedState(new ExpectedSlotStatus(slotStatus.getId(), STOPPED, installation.getAssignment()));
}
return builder.build();
}
});

return ImmutableList.copyOf(Iterables.concat(results));
}

private List<RemoteAgent> selectAgents(Predicate<AgentStatus> filter, Installation installation)
Expand Down
7 changes: 5 additions & 2 deletions ...hip-coordinator/src/main/java/io/airlift/airship/coordinator/CoordinatorSlotResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,18 @@ public class CoordinatorSlotResource

private final Coordinator coordinator;
private final Repository repository;
private final boolean allowDuplicateInstallationsOnAnAgent;

@Inject
public CoordinatorSlotResource(Coordinator coordinator, Repository repository)
public CoordinatorSlotResource(Coordinator coordinator, Repository repository, CoordinatorConfig config)
{
Preconditions.checkNotNull(coordinator, "coordinator must not be null");
Preconditions.checkNotNull(repository, "repository is null");
Preconditions.checkNotNull(config, "coordinatorConfig is null");

this.coordinator = coordinator;
this.repository = repository;
this.allowDuplicateInstallationsOnAnAgent = config.isAllowDuplicateInstallationsOnAnAgent();
}

@GET
Expand Down Expand Up @@ -102,7 +105,7 @@ public Response install(
Predicate<AgentStatus> agentFilter = AgentFilterBuilder.build(uriInfo,
transform(coordinator.getAgents(), idGetter()),
transform(coordinator.getAllSlotStatus(), uuidGetter()),
false,
allowDuplicateInstallationsOnAnAgent,
repository);
List<AgentStatus> agents = coordinator.getAgents(agentFilter);

Expand Down
2 changes: 1 addition & 1 deletion airship-coordinator/src/test/java/io/airlift/airship/coordinator/TestAdminResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void setUp()
new MockServiceInventory(),
new Duration(1, TimeUnit.DAYS),
false);
resource = new AdminResource(coordinator, repository);
resource = new AdminResource(coordinator, repository, new CoordinatorConfig());
}

@AfterMethod
Expand Down
5 changes: 3 additions & 2 deletions ...coordinator/src/test/java/io/airlift/airship/coordinator/TestCoordinatorSlotResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,21 @@ public void setUp()
throws Exception
{
NodeInfo nodeInfo = new NodeInfo("testing");
CoordinatorConfig config = new CoordinatorConfig().setStatusExpiration(new Duration(1, TimeUnit.DAYS));

repository = new TestingMavenRepository();

provisioner = new MockProvisioner();
coordinator = new Coordinator(nodeInfo,
new HttpServerInfo(new HttpServerConfig(), nodeInfo),
new CoordinatorConfig().setStatusExpiration(new Duration(1, TimeUnit.DAYS)),
config,
provisioner.getCoordinatorFactory(),
provisioner.getAgentFactory(),
repository,
provisioner,
new InMemoryStateManager(),
new MockServiceInventory());
resource = new CoordinatorSlotResource(coordinator, repository);
resource = new CoordinatorSlotResource(coordinator, repository, config);
}

@AfterMethod
Expand Down