[Part 15]: Resource Cluster Abstraction and an implementation based on Akka actor #166

sundargates · 2022-04-11T00:24:43Z

Context

Resource Cluster is a new abstraction to capture all the task executors that are registered with the mantis master.

Checklist

./gradlew build compiles code correctly
Added new tests where applicable
./gradlew test passes all tests
Extended README or added javadocs where applicable
Added copyright headers for new files from CONTRIBUTING.md

…n Akka actor

Andyz26 · 2022-04-11T15:39:40Z

...plane-server/src/main/java/io/mantisrx/server/master/persistence/IMantisStorageProvider.java

@@ -134,4 +136,8 @@ void storeAndUpdateWorkers(final IMantisWorkerMetadata existingWorker, final IMa
    List<String> initActiveVmAttributeValuesList() throws IOException;

    void setActiveVmAttributeValuesList(final List<String> vmAttributesList) throws IOException;
+
+    TaskExecutorRegistration getTaskExecutorFor(TaskExecutorID taskExecutorID) throws IOException;


Let's sync offline to see how to resolve the internal cass problem for these new data required.

Andyz26 · 2022-04-11T15:54:21Z

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

+            if (taskExecutorRegistration.isPresent()) {
+                sender().tell(taskExecutorRegistration.get(), self());
+            } else {
+                sender().tell(new Status.Failure(new Exception("")), self());


nit: error message?

Fixed. Thanks for spotting this.

...l-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterAkkaImpl.java

Andyz26 · 2022-04-11T16:03:43Z

...l-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterAkkaImpl.java

+            .thenApply(Ack.class::cast)
+            .toCompletableFuture()
+            .whenComplete((ack, dontCare) ->
+                mapper.onTaskExecutorDiscovered(clusterID, taskExecutorID));


What happens if there is an error from the future result?

it doesn't matter. I still want the association to be made.

codyrioux · 2022-04-11T17:13:17Z

...ne-server/src/main/java/io/mantisrx/server/master/store/SimpleCachedFileStorageProvider.java

@@ -521,4 +523,13 @@ public List<String> initActiveVmAttributeValuesList() throws IOException {
        }
    }

+    @Override
+    public TaskExecutorRegistration getTaskExecutorFor(TaskExecutorID taskExecutorID) throws IOException {
+        return null;


This SimpleCachedFileStorageProvider doesn't appear to do that the name implies. It looks a lot like the NoOp one.

Thanks for pointing it out. Honestly, I was not very impressed by that implementation and thought I could fix it later for OSS. But, I think it might be a good idea to keep this as-is for the time being and revisit it at a later point.

sundargates · 2022-04-11T19:54:54Z

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

+ */
+@ToString(of = {"clusterID"})
+@Slf4j
+class ResourceClusterActor extends AbstractActor {


This represents the main actor that needs to be reviewed fully for this change.

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

sundargates · 2022-04-11T19:55:38Z

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

+    private final ClusterID clusterID;
+    private final MantisJobStore mantisJobStore;
+
+    public static Props props(final ClusterID clusterID, final Duration heartbeatTimeout, Clock clock, RpcService rpcService) {


nit: package-private since the class is not exposed.

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

sundargates · 2022-04-11T20:08:40Z

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

+            TaskExecutorRegistration registration =
+                mantisJobStore.getTaskExecutor(request.getTaskExecutorID());
+            setupTaskExecutorStateIfNecessary(request.getTaskExecutorID());
+            self().tell(registration, self());
+            self().tell(
+                new TaskExecutorStatusChange(
+                    registration.getTaskExecutorID(),
+                    registration.getClusterID(),
+                    TaskExecutorReport.occupied(request.getWorkerId())),
+                self());
+            sender.tell(Ack.getInstance(), self());


write a test for this workflow.

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

calvin681 · 2022-04-11T21:42:58Z

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

+            matchedExecutor.get().getValue().onAssignment(request.getWorkerId());
+            sender().tell(matchedExecutor.get().getKey(), self());
+        } else {
+            sender().tell(new Status.Failure(new NoResourceAvailableException(


should we add a TODO here that we should expand the cluster?

sundargates · 2022-04-11T22:06:39Z

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java

+            if (state.isRegistered() && state.getGateway().isDone()) {
+                sender().tell(state.getGateway().join(), self());
+            } else {
+                sender().tell(new Status.Failure(new Exception("")), self());


more descriptive exception please.

[Part 15]: Resource Cluster Abstraction and an implementation based o…

4cf0736

…n Akka actor

sundargates requested review from codyrioux, nickmahilani, jeffchao, piygoyal, calvin681, Andyz26, hmitnflx, markcho and liuml07 as code owners April 11, 2022 00:24

Created fixture for DelegateClock and TestingRpcService.

c6b3991

Andyz26 reviewed Apr 11, 2022

View reviewed changes

...l-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterAkkaImpl.java Outdated Show resolved Hide resolved

Andyz26 reviewed Apr 11, 2022

View reviewed changes

sundargates added 3 commits April 11, 2022 09:52

Adding a test

8964de6

Refactoring resource cluster Akka impl.

0dbb53f

formatting code

d8396e0

codyrioux suggested changes Apr 11, 2022

View reviewed changes

sundargates added 2 commits April 11, 2022 12:42

Adding basic implementations for SimpleCachedFileStorageProvider

967883f

fixing compilation issues

74d502c

sundargates commented Apr 11, 2022

View reviewed changes

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java Outdated Show resolved Hide resolved

sundargates commented Apr 11, 2022

View reviewed changes

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java Show resolved Hide resolved

sundargates commented Apr 11, 2022

View reviewed changes

...trol-plane-server/src/main/java/io/mantisrx/master/resourcecluster/ResourceClusterActor.java Show resolved Hide resolved

sundargates added 2 commits April 11, 2022 13:15

Added a test to verify initialization lifecycle.

1de21da

Changed access modifiers to package-private.

0f60395

calvin681 reviewed Apr 11, 2022

View reviewed changes

sundargates commented Apr 11, 2022

View reviewed changes

Making sure assignment timeout is respected.

ceaa500

calvin681 approved these changes Apr 11, 2022

View reviewed changes

sundargates merged commit 53ac6fa into Netflix:master Apr 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Part 15]: Resource Cluster Abstraction and an implementation based on Akka actor #166

[Part 15]: Resource Cluster Abstraction and an implementation based on Akka actor #166

sundargates commented Apr 11, 2022

Andyz26 Apr 11, 2022

sundargates Apr 11, 2022

Andyz26 Apr 11, 2022

sundargates Apr 11, 2022

Andyz26 Apr 11, 2022

sundargates Apr 11, 2022

codyrioux Apr 11, 2022

sundargates Apr 11, 2022

sundargates Apr 11, 2022

sundargates Apr 11, 2022

sundargates Apr 11, 2022

sundargates Apr 11, 2022

calvin681 Apr 11, 2022

sundargates Apr 11, 2022

[Part 15]: Resource Cluster Abstraction and an implementation based on Akka actor #166

[Part 15]: Resource Cluster Abstraction and an implementation based on Akka actor #166

Conversation

sundargates commented Apr 11, 2022

Context

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment