Skip to content
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

Component creation transparency #63

Open
wants to merge 7 commits into
base: pg-merge
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 2 additions & 0 deletions src/main/java/org/hobbit/core/Constants.java
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,6 @@ private Constants() {

public static final TimeZone DEFAULT_TIME_ZONE = TimeZone.getTimeZone("GMT");

public static final String RABBIT_CONTAINER_SERVICE = "RABBIT_CONTAINER_SERVICE";

}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
import org.apache.jena.vocabulary.RDF;
import org.hobbit.core.Commands;
import org.hobbit.core.Constants;
import org.hobbit.core.containerservice.ContainerCreation;
import org.hobbit.core.containerservice.ContainerCreationFactory;
import org.hobbit.core.containerservice.RabbitMQContainerCreator;
import org.hobbit.core.rabbit.RabbitMQUtils;
import org.hobbit.utils.EnvVariables;
import org.hobbit.vocab.HOBBIT;
Expand Down Expand Up @@ -135,6 +138,10 @@ public abstract class AbstractBenchmarkController extends AbstractPlatformConnec
* The URI of the experiment.
*/
protected String experimentUri;
/**
* The instance to create container
*/
protected ContainerCreation containerCreation;

public AbstractBenchmarkController() {
defaultContainerType = Constants.CONTAINER_TYPE_BENCHMARK;
Expand All @@ -143,6 +150,7 @@ public AbstractBenchmarkController() {
@Override
public void init() throws Exception {
super.init();
containerCreation = ContainerCreationFactory.getContainerCreationObject(EnvVariables.getString(Constants.RABBIT_CONTAINER_SERVICE, LOGGER), this);
// benchmark controllers should be able to accept broadcasts
addCommandHeaderId(Constants.HOBBIT_SESSION_ID_FOR_BROADCASTS);

Expand Down Expand Up @@ -206,7 +214,7 @@ protected void createTaskGenerators(String taskGeneratorImageName, int numberOfT
* @param generatorIds
* set of generator container names
*/
private void createGenerator(String generatorImageName, int numberOfGenerators, String[] envVariables,
public void createGenerator(String generatorImageName, int numberOfGenerators, String[] envVariables,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please explain why you need to change the visibility of this method.

Set<String> generatorIds) {
String containerId;
String variables[] = envVariables != null ? Arrays.copyOf(envVariables, envVariables.length + 2)
Expand Down Expand Up @@ -608,4 +616,23 @@ protected void containerCrashed(String containerName) {
sendResultModel(resultModel);
System.exit(1);
}

public Set<String> getDataGenContainerIds() {
return dataGenContainerIds;
}

public Set<String> getTaskGenContainerIds() {
return taskGenContainerIds;
}

public String getEvalStoreContainerId() {
return evalStoreContainerId;
}

public void setEvalStoreContainerId(String evalStoreContainerId) {
this.evalStoreContainerId = evalStoreContainerId;
}



}
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,17 @@ public abstract class AbstractCommandReceivingComponent extends AbstractComponen
/**
* Name of this Docker container.
*/
private String containerName;
protected String containerName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have all the visibility modifiers been changed? It does not make sense to me. Please explain why it is necessary.

/**
* Name of the queue that is used to receive responses for messages that are
* sent via the command queue and for which an answer is expected.
*/
private String responseQueueName = null;
protected String responseQueueName = null;
/**
* Mapping of RabbitMQ's correlationIDs to Future objects corresponding
* to that RPC call.
*/
private Map<String, SettableFuture<String>> responseFutures = Collections.synchronizedMap(new LinkedHashMap<>());
protected Map<String, SettableFuture<String>> responseFutures = Collections.synchronizedMap(new LinkedHashMap<>());
/**
* Consumer of the queue that is used to receive responses for messages that
* are sent via the command queue and for which an answer is expected.
Expand Down Expand Up @@ -291,7 +291,7 @@ protected String createContainer(String imageName, String[] envVariables) {
* user-provided array of environment variables
* @return the extended array of environment variables
*/
protected String[] extendContainerEnvVariables(String[] envVariables) {
public String[] extendContainerEnvVariables(String[] envVariables) {
if (envVariables == null) {
envVariables = new String[0];
}
Expand Down Expand Up @@ -334,7 +334,7 @@ protected String[] extendContainerEnvVariables(String[] envVariables) {
* container
* @return the name of the container instance or null if an error occurred
*/
protected String createContainer(String imageName, String containerType, String[] envVariables) {
public String createContainer(String imageName, String containerType, String[] envVariables) {
return createContainer(imageName, containerType, envVariables, null);
}

Expand Down Expand Up @@ -461,7 +461,15 @@ protected Future<String> createContainerAsync(String imageName, String container
return null;
}

/**
public Map<String, SettableFuture<String>> getResponseFutures() {
return responseFutures;
}

public String getResponseQueueName() {
return responseQueueName;
}

/**
* This method sends a {@link Commands#DOCKER_CONTAINER_STOP} command to
* stop the container with the given id.
*
Expand All @@ -484,7 +492,7 @@ protected void stopContainer(String containerName) {
* @throws IOException
* if a communication problem occurs
*/
private void initResponseQueue() throws IOException {
public void initResponseQueue() throws IOException {
if (responseQueueName == null) {
responseQueueName = cmdChannel.queueDeclare().getQueue();
}
Expand Down Expand Up @@ -554,5 +562,28 @@ public void close() throws IOException {
}
super.close();
}

public String getContainerName() {
return containerName;
}

public void setContainerName(String containerName) {
this.containerName = containerName;
}

public Gson getGson() {
return gson;
}

public void setGson(Gson gson) {
this.gson = gson;
}

public void createDummyComponent(byte command, byte[] data, String sessionId, BasicProperties props) {
// TODO Auto-generated method stub

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't we discuss that this method is not necessary in this abstract class? Or is there a reason why it is still here?




}
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ public void init() throws Exception {
dataGenReceiver = DataReceiverImpl.builder().dataHandler(new DataHandler() {
@Override
public void handleData(byte[] data) {

Copy link
Contributor

Choose a reason for hiding this comment

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

If that is the only change in this class, it does not make sense to me to have it. Please try to avoid changes like that and revert the change here.

receiveGeneratedData(data);
}
}).maxParallelProcessedMsgs(maxParallelProcessedMsgs).queue(getFactoryForIncomingDataQueues(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.hobbit.core.containerservice;

import org.hobbit.core.components.AbstractCommandReceivingComponent;
/**
* ContainerCreation provides the facility to implement the functionalities
* to create {@link DirectContainerCreator} or {@link RabbitMQContainerCreator}
* @author altaf, sourabh, yamini, melisa
*
*/
public interface ContainerCreation {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add javadoc comments to all methods of this interface.


void createDataGenerators(String dataGeneratorImageName, int numberOfDataGenerators,
String[] envVariables, AbstractCommandReceivingComponent dummyComponent);

void createTaskGenerators(String taskGeneratorImageName, int numberOfTaskGenerators,
String[] envVariables, AbstractCommandReceivingComponent dummyComponent);

void createEvaluationStorage(String evalStorageImageName, String[] envVariables,
AbstractCommandReceivingComponent dummyComponent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why these three methods? It does not make much sense to me because of two different reasons:

  1. For the creation of containers, it doesn't matter which container is created. To this end, the factory should only provide the functionality to create any container regardless which container should be created.
  2. With this implementation, you are not able to create any other container. This limits the usage of the class a lot.

Apart from that, the container IDs should be returned by the method so that the requesting class can decide where it may want to store the ID.


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package org.hobbit.core.containerservice;

import org.hobbit.core.Constants;
import org.hobbit.core.components.AbstractBenchmarkController;

/**
* Factory that provides functionality to get an instance of {@link ContainerCreation}
* @author altaf, sourabh, yamini, melisa
*
*/
public class ContainerCreationFactory {

/**
* This method returns the instance of {@link RabbitMQContainerCreator} or {@link DirectContainerCreator}
* based on the environment variable {@link Constants#RABBIT_CONTAINER_SERVICE}
*/
public static ContainerCreation getContainerCreationObject(String isRabbitContainerService, AbstractBenchmarkController abstractBenchmarkController) {
if(isRabbitContainerService.equals("true")) {
return new RabbitMQContainerCreator(abstractBenchmarkController);
}
return new DirectContainerCreator(abstractBenchmarkController);
}

}
Loading