turcsanyip commented on a change in pull request #4821:
URL: https://github.com/apache/nifi/pull/4821#discussion_r577613897



##########
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/storage/BufferedNodeStatusStorage.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.controller.status.history.storage;
+
+import org.apache.commons.math3.util.Pair;
+import org.apache.nifi.controller.status.NodeStatus;
+import org.apache.nifi.controller.status.history.StatusHistory;
+
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Decorator in front of a {@link NodeStatusStorage}. It accumulates entries 
to store within an internal buffer until
+ * method {@link #persist()} is being called, when buffer content is drained 
and being sent to te decorated instance.
+ */
+public class BufferedNodeStatusStorage implements NodeStatusStorage, 
Persistable {
+    private final BlockingQueue<Pair<Date, NodeStatus>> queue = new 
LinkedBlockingQueue<>();
+    private final NodeStatusStorage payload;

Review comment:
       `BufferedNodeStatusStorage` and `QuestDbNodeStatusStorage` implement 
this `NodeStatusStorage` interface but they cannot be used interchangeably. In 
`EmbeddedQuestDbNodeStatusRepository` a `BufferedNodeStatusStorage` needed 
(because it implements `Persistable` too) and here it does not really make 
sense to use a buffered instance (and it just would not work).
   For this reason, it would be better to separate these two classes (eg. via 
removing `BufferedNodeStatusStorage` from the hierarchy).

##########
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/EmbeddedQuestDbStatusWriter.java
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.controller.status.history;
+
+import org.apache.nifi.controller.status.history.storage.Persistable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class EmbeddedQuestDbStatusWriter implements Runnable {

Review comment:
       There is no QuestDB specific logic in this class so it should be renamed 
to some more generic name.

##########
File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/questdb/QuestDbEntityWritingTemplate.java
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.controller.status.history.questdb;
+
+import io.questdb.cairo.TableWriter;
+import org.apache.commons.math3.util.Pair;
+
+import java.util.Collection;
+import java.util.Date;
+import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
+
+/**
+ * Writes entry to the database with the given measurement time.
+ *
+ * @param <E> Entry type.
+ */
+public class QuestDbEntityWritingTemplate<E> extends 
QuestDbWritingTemplate<Pair<Date, E>> {
+    private final BiConsumer<E, TableWriter.Row> fillRow;
+
+    /**
+     * @param tableName Name of the target table.
+     * @param fillRow Responsible for filling a row based on the entry.
+     */
+    public QuestDbEntityWritingTemplate(final String tableName, final 
BiConsumer<E, TableWriter.Row> fillRow) {
+        super(tableName);
+        this.fillRow = fillRow;
+    }
+
+    @Override
+    protected void addRows(final TableWriter tableWriter, final 
Collection<Pair<Date, E>> entries) {
+        entries.forEach(statusEntry -> {
+            final long measuredAt = 
TimeUnit.MILLISECONDS.toMicros(statusEntry.getFirst().getTime());

Review comment:
       Both `measuredAt` and `capturedAt` are used throughout the code. Not 
sure they have the same meaning in every place but it they do, the same term 
should be used to make it clear.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to