Github user bbende commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/120#discussion_r177765399
--- Diff:
minifi-c2/minifi-c2-framework/src/main/java/org/apache/nifi/minifi/c2/core/service/StandardC2Service.java
---
@@ -0,0 +1,494 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.core.service;
+
+import
org.apache.nifi.minifi.c2.api.provider.agent.AgentPersistenceProvider;
+import
org.apache.nifi.minifi.c2.api.provider.device.DevicePersistenceProvider;
+import
org.apache.nifi.minifi.c2.api.provider.operations.OperationPersistenceProvider;
+import org.apache.nifi.minifi.c2.core.exception.ResourceNotFoundException;
+import org.apache.nifi.minifi.c2.model.Agent;
+import org.apache.nifi.minifi.c2.model.AgentClass;
+import org.apache.nifi.minifi.c2.model.AgentManifest;
+import org.apache.nifi.minifi.c2.model.Device;
+import org.apache.nifi.minifi.c2.model.OperationRequest;
+import org.apache.nifi.minifi.c2.model.OperationState;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.ConstraintViolationException;
+import javax.validation.Validator;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+@Service
+public class StandardC2Service implements C2Service {
+
+ private static final Logger logger =
LoggerFactory.getLogger(StandardC2Service.class);
+
+ private final AgentPersistenceProvider agentPersistenceProvider;
+ private final DevicePersistenceProvider devicePersistenceProvider;
+ private final OperationPersistenceProvider
operationPersistenceProvider;
+ private final Validator validator;
+
+ private final ReentrantReadWriteLock lock = new
ReentrantReadWriteLock();
--- End diff --
Do we think the read/write lock approach is too limiting for C2?
I know we did this in registry to ensure consistency when interacting with
more than one data store, but the trade-off is that it reduces the concurrency
a lot because all threads are blocked while one thread has the write lock.
I don't know the level of concurrent requests we are anticipating here so
it may be a non-issue, but just wanted to throw it out there. This also may be
totally fine for initial implementation, and we can modify/improve later.
I also haven't read through all the methods below yet, but if there isn't a
case where a single method performs operations across multiple persistence
providers, then we could probably make the locks more granular and have a lock
for each persistence provider so that agent calls are not blocked by device
calls, etc.
---
