Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory closed pull request #8152: NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository URL: https://github.com/apache/nifi/pull/8152 -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1482432782
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/test/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerTest.java:
##
@@ -0,0 +1,397 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.rollover.RolloverStrategy;
+import org.apache.nifi.questdb.util.Event;
+import org.apache.nifi.questdb.util.QuestDbTestUtil;
+import org.apache.nifi.util.file.FileUtils;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.StreamSupport;
+
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT2_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT2_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.SELECT_QUERY;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getRandomTestData;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getTestData;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class EmbeddedDatabaseManagerTest extends EmbeddedQuestDbTest {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManagerTest.class);
+private static final int DAYS_TO_KEEP_EVENT = 1;
+private static final String NON_EXISTING_PLACE = SystemUtils.IS_OS_WINDOWS
? "T:/nonExistingPlace" : "/nonExistingPlace";
+
+@Test
+public void testAcquiringWithoutInitialization() {
+final EmbeddedDatabaseManager testSubject = new
EmbeddedDatabaseManager(new SimpleEmbeddedDatabaseManagerContext());
+assertThrows(IllegalStateException.class, () ->
testSubject.acquireClient());
+}
+
+@Test
+public void testHappyPath() throws DatabaseException {
+final List testData = getTestData();
+final DatabaseManager testSubject = getTestSubject();
+assertDatabaseFolderIsNotEmpty();
+
+final Client client = testSubject.acquireClient();
+
+client.insert(EVENT_TABLE_NAME,
QuestDbTestUtil.getEventTableDataSource(testData));
+final List result = client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING));
+
+assertIterableEquals(testData, result);
+
+testSubject.close();
+
+// Even if the client itself is not connected, manager prevents client
to reach database after closing
+assertFalse(client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING)).iterator().hasNext());
+}
+
+@Test
+public void testRollover() throws DatabaseException, InterruptedException {
+final List testData = new ArrayList<>();
+testData.add(new Event(Instant.now().minus((DAYS_TO_KEEP_EVENT + 1),
ChronoUnit.DAYS), "A", 1));
+testData.add(new Event(Instant.now(), "B",
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1481596508
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/test/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerTest.java:
##
@@ -0,0 +1,397 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.rollover.RolloverStrategy;
+import org.apache.nifi.questdb.util.Event;
+import org.apache.nifi.questdb.util.QuestDbTestUtil;
+import org.apache.nifi.util.file.FileUtils;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.StreamSupport;
+
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT2_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT2_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.SELECT_QUERY;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getRandomTestData;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getTestData;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class EmbeddedDatabaseManagerTest extends EmbeddedQuestDbTest {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManagerTest.class);
+private static final int DAYS_TO_KEEP_EVENT = 1;
+private static final String NON_EXISTING_PLACE = SystemUtils.IS_OS_WINDOWS
? "T:/nonExistingPlace" : "/nonExistingPlace";
+
+@Test
+public void testAcquiringWithoutInitialization() {
+final EmbeddedDatabaseManager testSubject = new
EmbeddedDatabaseManager(new SimpleEmbeddedDatabaseManagerContext());
+assertThrows(IllegalStateException.class, () ->
testSubject.acquireClient());
+}
+
+@Test
+public void testHappyPath() throws DatabaseException {
+final List testData = getTestData();
+final DatabaseManager testSubject = getTestSubject();
+assertDatabaseFolderIsNotEmpty();
+
+final Client client = testSubject.acquireClient();
+
+client.insert(EVENT_TABLE_NAME,
QuestDbTestUtil.getEventTableDataSource(testData));
+final List result = client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING));
+
+assertIterableEquals(testData, result);
+
+testSubject.close();
+
+// Even if the client itself is not connected, manager prevents client
to reach database after closing
+assertFalse(client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING)).iterator().hasNext());
+}
+
+@Test
+public void testRollover() throws DatabaseException, InterruptedException {
+final List testData = new ArrayList<>();
+testData.add(new Event(Instant.now().minus((DAYS_TO_KEEP_EVENT + 1),
ChronoUnit.DAYS), "A", 1));
+testData.add(new Event(Instant.now(), "B", 2));
+
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1481594813
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/test/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerTest.java:
##
@@ -0,0 +1,397 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.rollover.RolloverStrategy;
+import org.apache.nifi.questdb.util.Event;
+import org.apache.nifi.questdb.util.QuestDbTestUtil;
+import org.apache.nifi.util.file.FileUtils;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.StreamSupport;
+
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT2_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT2_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.SELECT_QUERY;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getRandomTestData;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getTestData;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class EmbeddedDatabaseManagerTest extends EmbeddedQuestDbTest {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManagerTest.class);
+private static final int DAYS_TO_KEEP_EVENT = 1;
+private static final String NON_EXISTING_PLACE = SystemUtils.IS_OS_WINDOWS
? "T:/nonExistingPlace" : "/nonExistingPlace";
+
+@Test
+public void testAcquiringWithoutInitialization() {
+final EmbeddedDatabaseManager testSubject = new
EmbeddedDatabaseManager(new SimpleEmbeddedDatabaseManagerContext());
+assertThrows(IllegalStateException.class, () ->
testSubject.acquireClient());
+}
+
+@Test
+public void testHappyPath() throws DatabaseException {
+final List testData = getTestData();
+final DatabaseManager testSubject = getTestSubject();
+assertDatabaseFolderIsNotEmpty();
+
+final Client client = testSubject.acquireClient();
+
+client.insert(EVENT_TABLE_NAME,
QuestDbTestUtil.getEventTableDataSource(testData));
+final List result = client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING));
+
+assertIterableEquals(testData, result);
+
+testSubject.close();
+
+// Even if the client itself is not connected, manager prevents client
to reach database after closing
+assertFalse(client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING)).iterator().hasNext());
+}
+
+@Test
+public void testRollover() throws DatabaseException, InterruptedException {
+final List testData = new ArrayList<>();
+testData.add(new Event(Instant.now().minus((DAYS_TO_KEEP_EVENT + 1),
ChronoUnit.DAYS), "A", 1));
+testData.add(new Event(Instant.now(), "B", 2));
+
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1479952728
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/test/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerTest.java:
##
@@ -0,0 +1,397 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.rollover.RolloverStrategy;
+import org.apache.nifi.questdb.util.Event;
+import org.apache.nifi.questdb.util.QuestDbTestUtil;
+import org.apache.nifi.util.file.FileUtils;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.StreamSupport;
+
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT2_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT2_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.SELECT_QUERY;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getRandomTestData;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getTestData;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class EmbeddedDatabaseManagerTest extends EmbeddedQuestDbTest {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManagerTest.class);
+private static final int DAYS_TO_KEEP_EVENT = 1;
+private static final String NON_EXISTING_PLACE = SystemUtils.IS_OS_WINDOWS
? "T:/nonExistingPlace" : "/nonExistingPlace";
+
+@Test
+public void testAcquiringWithoutInitialization() {
+final EmbeddedDatabaseManager testSubject = new
EmbeddedDatabaseManager(new SimpleEmbeddedDatabaseManagerContext());
+assertThrows(IllegalStateException.class, () ->
testSubject.acquireClient());
+}
+
+@Test
+public void testHappyPath() throws DatabaseException {
+final List testData = getTestData();
+final DatabaseManager testSubject = getTestSubject();
+assertDatabaseFolderIsNotEmpty();
+
+final Client client = testSubject.acquireClient();
+
+client.insert(EVENT_TABLE_NAME,
QuestDbTestUtil.getEventTableDataSource(testData));
+final List result = client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING));
+
+assertIterableEquals(testData, result);
+
+testSubject.close();
+
+// Even if the client itself is not connected, manager prevents client
to reach database after closing
+assertFalse(client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING)).iterator().hasNext());
+}
+
+@Test
+public void testRollover() throws DatabaseException, InterruptedException {
+final List testData = new ArrayList<>();
+testData.add(new Event(Instant.now().minus((DAYS_TO_KEEP_EVENT + 1),
ChronoUnit.DAYS), "A", 1));
+testData.add(new Event(Instant.now(), "B",
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1479945239
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/test/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerTest.java:
##
@@ -0,0 +1,397 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.rollover.RolloverStrategy;
+import org.apache.nifi.questdb.util.Event;
+import org.apache.nifi.questdb.util.QuestDbTestUtil;
+import org.apache.nifi.util.file.FileUtils;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.StreamSupport;
+
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT2_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT2_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.SELECT_QUERY;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getRandomTestData;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getTestData;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class EmbeddedDatabaseManagerTest extends EmbeddedQuestDbTest {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManagerTest.class);
+private static final int DAYS_TO_KEEP_EVENT = 1;
+private static final String NON_EXISTING_PLACE = SystemUtils.IS_OS_WINDOWS
? "T:/nonExistingPlace" : "/nonExistingPlace";
+
+@Test
+public void testAcquiringWithoutInitialization() {
+final EmbeddedDatabaseManager testSubject = new
EmbeddedDatabaseManager(new SimpleEmbeddedDatabaseManagerContext());
+assertThrows(IllegalStateException.class, () ->
testSubject.acquireClient());
+}
+
+@Test
+public void testHappyPath() throws DatabaseException {
+final List testData = getTestData();
+final DatabaseManager testSubject = getTestSubject();
+assertDatabaseFolderIsNotEmpty();
+
+final Client client = testSubject.acquireClient();
+
+client.insert(EVENT_TABLE_NAME,
QuestDbTestUtil.getEventTableDataSource(testData));
+final List result = client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING));
+
+assertIterableEquals(testData, result);
+
+testSubject.close();
+
+// Even if the client itself is not connected, manager prevents client
to reach database after closing
+assertFalse(client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING)).iterator().hasNext());
+}
+
+@Test
+public void testRollover() throws DatabaseException, InterruptedException {
+final List testData = new ArrayList<>();
+testData.add(new Event(Instant.now().minus((DAYS_TO_KEEP_EVENT + 1),
ChronoUnit.DAYS), "A", 1));
+testData.add(new Event(Instant.now(), "B",
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1479876942
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManager.java:
##
@@ -0,0 +1,294 @@
+/*
+ * 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.questdb.embedded;
+
+import io.questdb.cairo.CairoConfiguration;
+import io.questdb.cairo.CairoEngine;
+import io.questdb.cairo.DefaultCairoConfiguration;
+import io.questdb.cairo.TableToken;
+import io.questdb.cairo.sql.TableRecordMetadata;
+import org.apache.commons.lang3.concurrent.BasicThreadFactory;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.util.file.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.stream.Collectors;
+
+final class EmbeddedDatabaseManager implements DatabaseManager {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManager.class);
+
+private final String id = UUID.randomUUID().toString();
+private final AtomicReference state = new
AtomicReference(EmbeddedDatabaseManagerStatus.UNINITIALIZED);
+private final ReadWriteLock databaseStructureLock = new
ReentrantReadWriteLock();
+private final EmbeddedDatabaseManagerContext context;
+private final AtomicReference engine = new
AtomicReference<>();
+private final List> scheduledFutures = new
ArrayList<>();
+private final ScheduledExecutorService scheduledExecutorService = Executors
+.newScheduledThreadPool(2, new
BasicThreadFactory.Builder().namingPattern("EmbeddedQuestDbManagerWorker-" + id
+ "-%d").build());
+
+EmbeddedDatabaseManager(final EmbeddedDatabaseManagerContext context) {
+this.context = context;
+}
+
+@Override
+public void init() {
+if (state.get() != EmbeddedDatabaseManagerStatus.UNINITIALIZED) {
+throw new IllegalStateException("Manager is already initialized");
+}
+
+ensureDatabaseIsReady();
+startRollover();
+}
+
+private void ensureDatabaseIsReady() {
+boolean successful = false;
+
+try {
+databaseStructureLock.writeLock().lock();
+state.set(EmbeddedDatabaseManagerStatus.REPAIRING);
+
+try {
+ensurePersistLocationIsAccessible();
+ensureConnectionEstablished();
+ensureTablesAreInPlaceAndHealthy();
+successful = true;
+} catch (final CorruptedDatabaseException e) {
+boolean couldMoveOldToBackup = false;
+
+try {
+LOGGER.error("Database is corrupted. Recreation is
triggered. Manager tries to move corrupted database files to the backup
location: {}", context.getBackupLocation(), e);
+final File backupFolder = new
File(context.getBackupLocationAsPath().toFile(), "backup_" +
System.currentTimeMillis());
+
FileUtils.ensureDirectoryExistAndCanAccess(context.getBackupLocationAsPath().toFile());
+Files.move(context.getPersistLocationAsPath(),
backupFolder.toPath());
+couldMoveOldToBackup = true;
+} catch (IOException ex) {
+LOGGER.error("Could not create backup", ex);
+}
+
+if (!couldMoveOldToBackup) {
+try {
+
FileUtils.deleteFile(context.getPersistLocationAsPath().toFile(), true);
+
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1479260621
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManager.java:
##
@@ -0,0 +1,294 @@
+/*
+ * 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.questdb.embedded;
+
+import io.questdb.cairo.CairoConfiguration;
+import io.questdb.cairo.CairoEngine;
+import io.questdb.cairo.DefaultCairoConfiguration;
+import io.questdb.cairo.TableToken;
+import io.questdb.cairo.sql.TableRecordMetadata;
+import org.apache.commons.lang3.concurrent.BasicThreadFactory;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.util.file.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.stream.Collectors;
+
+final class EmbeddedDatabaseManager implements DatabaseManager {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManager.class);
+
+private final String id = UUID.randomUUID().toString();
+private final AtomicReference state = new
AtomicReference(EmbeddedDatabaseManagerStatus.UNINITIALIZED);
+private final ReadWriteLock databaseStructureLock = new
ReentrantReadWriteLock();
+private final EmbeddedDatabaseManagerContext context;
+private final AtomicReference engine = new
AtomicReference<>();
+private final List> scheduledFutures = new
ArrayList<>();
+private final ScheduledExecutorService scheduledExecutorService = Executors
+.newScheduledThreadPool(2, new
BasicThreadFactory.Builder().namingPattern("EmbeddedQuestDbManagerWorker-" + id
+ "-%d").build());
+
+EmbeddedDatabaseManager(final EmbeddedDatabaseManagerContext context) {
+this.context = context;
+}
+
+@Override
+public void init() {
+if (state.get() != EmbeddedDatabaseManagerStatus.UNINITIALIZED) {
+throw new IllegalStateException("Manager is already initialized");
+}
+
+ensureDatabaseIsReady();
+startRollover();
+}
+
+private void ensureDatabaseIsReady() {
+boolean successful = false;
+
+try {
+databaseStructureLock.writeLock().lock();
+state.set(EmbeddedDatabaseManagerStatus.REPAIRING);
+
+try {
+ensurePersistLocationIsAccessible();
+ensureConnectionEstablished();
+ensureTablesAreInPlaceAndHealthy();
+successful = true;
+} catch (final CorruptedDatabaseException e) {
+boolean couldMoveOldToBackup = false;
+
+try {
+LOGGER.error("Database is corrupted. Recreation is
triggered. Manager tries to move corrupted database files to the backup
location: {}", context.getBackupLocation(), e);
+final File backupFolder = new
File(context.getBackupLocationAsPath().toFile(), "backup_" +
System.currentTimeMillis());
+
FileUtils.ensureDirectoryExistAndCanAccess(context.getBackupLocationAsPath().toFile());
+Files.move(context.getPersistLocationAsPath(),
backupFolder.toPath());
+couldMoveOldToBackup = true;
+} catch (IOException ex) {
+LOGGER.error("Could not create backup", ex);
+}
+
+if (!couldMoveOldToBackup) {
+try {
+
FileUtils.deleteFile(context.getPersistLocationAsPath().toFile(), true);
+
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1479237840
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/test/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerTest.java:
##
@@ -0,0 +1,397 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.rollover.RolloverStrategy;
+import org.apache.nifi.questdb.util.Event;
+import org.apache.nifi.questdb.util.QuestDbTestUtil;
+import org.apache.nifi.util.file.FileUtils;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.StreamSupport;
+
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT2_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT2_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.SELECT_QUERY;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getRandomTestData;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getTestData;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class EmbeddedDatabaseManagerTest extends EmbeddedQuestDbTest {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManagerTest.class);
+private static final int DAYS_TO_KEEP_EVENT = 1;
+private static final String NON_EXISTING_PLACE = SystemUtils.IS_OS_WINDOWS
? "T:/nonExistingPlace" : "/nonExistingPlace";
Review Comment:
The case `testFallsBackToDummyWhenCannotEnsureDatabaseHealth` still uses
this. Others usages are guarded by `@DisabledOnOs`
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1478675324
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/test/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerTest.java:
##
@@ -0,0 +1,397 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.rollover.RolloverStrategy;
+import org.apache.nifi.questdb.util.Event;
+import org.apache.nifi.questdb.util.QuestDbTestUtil;
+import org.apache.nifi.util.file.FileUtils;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.StreamSupport;
+
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT2_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT2_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.SELECT_QUERY;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getRandomTestData;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getTestData;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class EmbeddedDatabaseManagerTest extends EmbeddedQuestDbTest {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManagerTest.class);
+private static final int DAYS_TO_KEEP_EVENT = 1;
+private static final String NON_EXISTING_PLACE = SystemUtils.IS_OS_WINDOWS
? "T:/nonExistingPlace" : "/nonExistingPlace";
+
+@Test
+public void testAcquiringWithoutInitialization() {
+final EmbeddedDatabaseManager testSubject = new
EmbeddedDatabaseManager(new SimpleEmbeddedDatabaseManagerContext());
+assertThrows(IllegalStateException.class, () ->
testSubject.acquireClient());
+}
+
+@Test
+public void testHappyPath() throws DatabaseException {
+final List testData = getTestData();
+final DatabaseManager testSubject = getTestSubject();
+assertDatabaseFolderIsNotEmpty();
+
+final Client client = testSubject.acquireClient();
+
+client.insert(EVENT_TABLE_NAME,
QuestDbTestUtil.getEventTableDataSource(testData));
+final List result = client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING));
+
+assertIterableEquals(testData, result);
+
+testSubject.close();
+
+// Even if the client itself is not connected, manager prevents client
to reach database after closing
+assertFalse(client.query(SELECT_QUERY,
RequestMapping.getResultProcessor(QuestDbTestUtil.EVENT_TABLE_REQUEST_MAPPING)).iterator().hasNext());
+}
+
+@Test
+public void testRollover() throws DatabaseException, InterruptedException {
+final List testData = new ArrayList<>();
+testData.add(new Event(Instant.now().minus((DAYS_TO_KEEP_EVENT + 1),
ChronoUnit.DAYS), "A", 1));
+testData.add(new Event(Instant.now(), "B",
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1476061099
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/BufferedStatusHistoryStorage.java:
##
@@ -0,0 +1,190 @@
+/*
+ * 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 org.apache.commons.lang3.concurrent.BasicThreadFactory;
+import org.apache.nifi.controller.status.ConnectionStatus;
+import org.apache.nifi.controller.status.NodeStatus;
+import org.apache.nifi.controller.status.ProcessGroupStatus;
+import org.apache.nifi.controller.status.ProcessorStatus;
+import org.apache.nifi.controller.status.RemoteProcessGroupStatus;
+import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
+import org.apache.nifi.controller.status.history.StatusSnapshot;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Date;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Executors;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
+
+final class BufferedStatusHistoryStorage implements StatusHistoryStorage {
+private static final Logger LOGGER =
LoggerFactory.getLogger(BufferedStatusHistoryStorage.class);
+
+private final String id = UUID.randomUUID().toString();
+private final List> scheduledFutures = new
ArrayList<>();
+private final ScheduledExecutorService scheduledExecutorService = Executors
+.newScheduledThreadPool(1, new
BasicThreadFactory.Builder().namingPattern("BufferedStatusHistoryStorage-" + id
+ "-%d").build());
+
+private final StatusHistoryStorage storage;
+private final long persistFrequencyInMs;
+private final int persistBatchSize;
+
+private final BlockingQueue> nodeStatusQueue =
new LinkedBlockingQueue<>();
+private final BlockingQueue>
garbageCollectionStatusQueue = new LinkedBlockingQueue<>();
+private final BlockingQueue>
processGroupStatusQueue = new LinkedBlockingQueue<>();
+private final BlockingQueue>
connectionStatusQueue = new LinkedBlockingQueue<>();
+private final BlockingQueue>
remoteProcessGroupStatusQueue = new LinkedBlockingQueue<>();
+private final BlockingQueue>
processorStatusQueue = new LinkedBlockingQueue<>();
+
+public BufferedStatusHistoryStorage(final StatusHistoryStorage storage,
final long persistFrequencyInMs, final int persistBatchSize) {
+this.storage = storage;
+this.persistFrequencyInMs = persistFrequencyInMs;
+this.persistBatchSize = persistBatchSize;
+}
+
+@Override
+public void init() {
+storage.init();
+final ScheduledFuture future =
scheduledExecutorService.scheduleWithFixedDelay(
+new BufferedStatusHistoryStorageWorker(),
persistFrequencyInMs, persistFrequencyInMs, TimeUnit.MILLISECONDS);
+scheduledFutures.add(future);
+LOGGER.info("Flushing is initiated");
+}
+
+@Override
+public void close() {
+storage.close();
+LOGGER.debug("Flushing shutdown started");
+int cancelCompleted = 0;
+int cancelFailed = 0;
+
+for (final ScheduledFuture scheduledFuture : scheduledFutures) {
+final boolean cancelled = scheduledFuture.cancel(true);
+if (cancelled) {
+cancelCompleted++;
+} else {
+cancelFailed++;
+}
+}
+
+LOGGER.debug("Flushing shutdown task cancellation status: completed
[{}] failed [{}]", cancelCompleted, cancelFailed);
+final List tasks = scheduledExecutorService.shutdownNow();
+LOGGER.debug(" Scheduled Task Service shutdown remaining tasks [{}]",
tasks.size());
Review Comment:
```suggestion
LOGGER.debug("Scheduled Task Service shutdown remaining tasks [{}]",
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8152: URL: https://github.com/apache/nifi/pull/8152#issuecomment-1908952704 Notable changes: - CapturedAt database fields were renamed to captured - I found the reason of the remaining test files and fixed it - I adjusted the NAR structure, now it is inherited from framework again, fixing an instance of issue - With this I renamed the NAR. I would prefer the more generic `nifi-questdb-nar` as name but with the framework as parent we would have challenges with processors in the NAR. So probably if we want to add them, it will be a separate NAR thus the more specific name is beneficial. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on PR #8152: URL: https://github.com/apache/nifi/pull/8152#issuecomment-1907356923 > > Thanks for making the updates @simonbence. > > On closer review of the test changes, there is still a problem with manager tests not cleaning up temporary files on completion. Disabling the methods on Windows masks this problem, but running the tests on macOS shows `junit` directories left behind after completion. I recommend removing the `DisabledOnOS` annotations and tracking down the source of the problem. Whether in the test class, or the implementation itself, temporary files should not remain after successful test completion. > > I also noticed several QuestDB table definitions that still use `capturedAt` as the column name, so this should be checked across the set of changes and updated. > > I take a look on the leftover temporary files. > > As for disable: It was a deliberate choice after looking into the reasons. I picked adding them due to the test setup, not because of production reasons. In order to emulate corrupted database files I directly touch the files and write some gibberish into that. This is something normally would happened outside of our code, most reasonably would happen within the database. I did not found other way to trigger this however Windows does not let one to touch a file is under lock by an other process. If there is an other way to deliberatly corrupt the database (not directly working with the underlying files) than it could work out, but I am not aware of such a method. Thanks for looking into the details. Manual file changes for the test methods seems like a reasonable strategy, so if you can track down and resolve the leftover files issue, I would expect that should also resolve the test failures on Windows. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8152: URL: https://github.com/apache/nifi/pull/8152#issuecomment-1906081709 > Thanks for making the updates @simonbence. > > On closer review of the test changes, there is still a problem with manager tests not cleaning up temporary files on completion. Disabling the methods on Windows masks this problem, but running the tests on macOS shows `junit` directories left behind after completion. I recommend removing the `DisabledOnOS` annotations and tracking down the source of the problem. Whether in the test class, or the implementation itself, temporary files should not remain after successful test completion. > > I also noticed several QuestDB table definitions that still use `capturedAt` as the column name, so this should be checked across the set of changes and updated. I take a look on the leftover temporary files. As for disable: It was a deliberate choice after looking into the reasons. I picked adding them due to the test setup, not because of production reasons. In order to emulate corrupted database files I directly touch the files and write some gibberish into that. This is something normally would happened outside of our code, most reasonably would happen within the database. I did not found other way to trigger this however Windows does not let one to touch a file is under lock by an other process. If there is an other way to deliberatly corrupt the database (not directly working with the underlying files) than it could work out, but I am not aware of such a method. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1463269610
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/pom.xml:
##
@@ -0,0 +1,66 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
https://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache.nifi
+nifi-questdb-bundle
+2.0.0-SNAPSHOT
+
+nifi-questdb
Review Comment:
The logging output from test classes is very verbose. It looks like the
`log-stdout.conf` needs to be copied over to the `test/resources` directory of
this module to avoid excessive output.
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/pom.xml:
##
@@ -0,0 +1,66 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
https://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache.nifi
+nifi-questdb-bundle
+2.0.0-SNAPSHOT
+
+nifi-questdb
+
+
+
+org.apache.nifi
+nifi-utils
+2.0.0-SNAPSHOT
+
+
+
+org.apache.commons
+commons-lang3
+
+
+commons-io
+commons-io
+
Review Comment:
This dependency does not appear to be used and should be removed if not
needed.
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/test/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerTest.java:
##
@@ -0,0 +1,403 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.rollover.RolloverStrategy;
+import org.apache.nifi.questdb.util.Event;
+import org.apache.nifi.questdb.util.QuestDbTestUtil;
+import org.apache.nifi.util.file.FileUtils;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.StreamSupport;
+
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT2_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.CREATE_EVENT_TABLE;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT2_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.EVENT_TABLE_NAME;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.SELECT_QUERY;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getRandomTestData;
+import static org.apache.nifi.questdb.util.QuestDbTestUtil.getTestData;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+public class EmbeddedDatabaseManagerTest extends EmbeddedQuestDbTest {
Review Comment:
This class leaves `junit` temporary directories around after completion.
Disabling some methods on Windows masks the fundamental problem of the test not
cleaning up after itself, so this needs more evaluation to improve the behavior.
--
This is an automated message from
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8152: URL: https://github.com/apache/nifi/pull/8152#issuecomment-1904912180 The failed checks look to be in connection with Python processors and minify -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8152: URL: https://github.com/apache/nifi/pull/8152#issuecomment-1900596724 > > > Thanks for making the adjustments, this looks cleaner with the move to `spring-retry`. > > > I noted a few minor things, but also there appears to be a file-locking problem resulting in test failures on Windows. Linux and macOS are generally more tolerant in this area, but it highlights either a test problem, or a problem with open files in the runtime class. See the Windows build for more details. > > > ``` > > > Error: org.apache.nifi.questdb.embedded.EmbeddedClientTest.testInsertAndQuery -- Time elapsed: 0.121 s <<< ERROR! > > > java.io.IOException: Failed to delete temp directory C:\Users\RUNNER~1\AppData\Local\Temp\junit16271263950658316199. The following paths could not be deleted (see suppressed exceptions for details): , event, event\2024-01-14, event\2024-01-14\capturedAt.d, event\2024-01-14\subject.d, event\2024-01-14\value.d, event\subject.c, event\subject.k, event\subject.o, event\subject.v, event\_cv, event\_meta, event\_txn, event\_txn_scoreboard, tables.d.0, _tab_index.d > > > ``` > > > > > > It looks like it has something to do with the `@TempDir` annotation. I am looking after it. > > Thanks for looking into it. The `@TempDir` annotation is useful because JUnit will automatically clear the directory after running a test method, so those types of locking errors usually indicate that there is some other thread which has not closed a file stream. This was my idea as well. I will add some explicit tearing down to see if it is sufficient for the TempDir -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on PR #8152: URL: https://github.com/apache/nifi/pull/8152#issuecomment-1900590396 > > Thanks for making the adjustments, this looks cleaner with the move to `spring-retry`. > > I noted a few minor things, but also there appears to be a file-locking problem resulting in test failures on Windows. Linux and macOS are generally more tolerant in this area, but it highlights either a test problem, or a problem with open files in the runtime class. See the Windows build for more details. > > ``` > > Error: org.apache.nifi.questdb.embedded.EmbeddedClientTest.testInsertAndQuery -- Time elapsed: 0.121 s <<< ERROR! > > java.io.IOException: Failed to delete temp directory C:\Users\RUNNER~1\AppData\Local\Temp\junit16271263950658316199. The following paths could not be deleted (see suppressed exceptions for details): , event, event\2024-01-14, event\2024-01-14\capturedAt.d, event\2024-01-14\subject.d, event\2024-01-14\value.d, event\subject.c, event\subject.k, event\subject.o, event\subject.v, event\_cv, event\_meta, event\_txn, event\_txn_scoreboard, tables.d.0, _tab_index.d > > ``` > > It looks like it has something to do with the `@TempDir` annotation. I am looking after it. Thanks for looking into it. The `@TempDir` annotation is useful because JUnit will automatically clear the directory after running a test method, so those types of locking errors usually indicate that there is some other thread which has not closed a file stream. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8152: URL: https://github.com/apache/nifi/pull/8152#issuecomment-1900554189 > Thanks for making the adjustments, this looks cleaner with the move to `spring-retry`. > > I noted a few minor things, but also there appears to be a file-locking problem resulting in test failures on Windows. Linux and macOS are generally more tolerant in this area, but it highlights either a test problem, or a problem with open files in the runtime class. See the Windows build for more details. > > ``` > Error: org.apache.nifi.questdb.embedded.EmbeddedClientTest.testInsertAndQuery -- Time elapsed: 0.121 s <<< ERROR! > java.io.IOException: Failed to delete temp directory C:\Users\RUNNER~1\AppData\Local\Temp\junit16271263950658316199. The following paths could not be deleted (see suppressed exceptions for details): , event, event\2024-01-14, event\2024-01-14\capturedAt.d, event\2024-01-14\subject.d, event\2024-01-14\value.d, event\subject.c, event\subject.k, event\subject.o, event\subject.v, event\_cv, event\_meta, event\_txn, event\_txn_scoreboard, tables.d.0, _tab_index.d > ``` It looks like it has something to do with the `@TempDir` annotation. I am looking after it. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152: URL: https://github.com/apache/nifi/pull/8152#discussion_r1458987461 ## nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/pom.xml: ## @@ -0,0 +1,67 @@ + + +http://maven.apache.org/POM/4.0.0; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd;> +4.0.0 + +org.apache.nifi +nifi-questdb-bundle +2.0.0-SNAPSHOT + +nifi-questdb + + + +org.apache.nifi +nifi-utils +2.0.0-SNAPSHOT + + + +org.apache.commons +commons-lang3 + + +commons-io +commons-io + + +org.slf4j +slf4j-api + + +org.questdb +questdb +7.3.7 + + +org.mockito +mockito-junit-jupiter + + + + +org.springframework +spring-core +6.1.2 Review Comment: `spring-retry` needs it and it is not a transitive for that. I removed the version that is a good point -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1458758879
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/rollover/DeleteOldRolloverStrategy.java:
##
@@ -0,0 +1,95 @@
+/*
+ * 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.questdb.rollover;
+
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.QueryResultProcessor;
+import org.apache.nifi.questdb.QueryRowContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.function.Supplier;
+
+final class DeleteOldRolloverStrategy implements RolloverStrategy {
+private static final Logger LOGGER =
LoggerFactory.getLogger(DeleteOldRolloverStrategy.class);
+private static final DateTimeFormatter DATE_FORMATTER =
DateTimeFormatter.ofPattern("-MM-dd").withZone(ZoneOffset.UTC);
Review Comment:
If I remember correctly (but this decision was made during the original
implementation) there was a concern for nodes in different time zones.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1458179457
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/pom.xml:
##
@@ -0,0 +1,67 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
https://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache.nifi
+nifi-questdb-bundle
+2.0.0-SNAPSHOT
+
+nifi-questdb
+
+
+
+org.apache.nifi
+nifi-utils
+2.0.0-SNAPSHOT
+
+
+
+org.apache.commons
+commons-lang3
+
+
+commons-io
+commons-io
+
+
+org.slf4j
+slf4j-api
+
+
+org.questdb
+questdb
+7.3.7
+
+
+org.mockito
+mockito-junit-jupiter
+
+
+
+
+org.springframework
+spring-core
+6.1.2
Review Comment:
This version can be removed because it is managed at the root pom.xml. Is
this dependency required?
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/rollover/DeleteOldRolloverStrategy.java:
##
@@ -0,0 +1,95 @@
+/*
+ * 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.questdb.rollover;
+
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.QueryResultProcessor;
+import org.apache.nifi.questdb.QueryRowContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.function.Supplier;
+
+final class DeleteOldRolloverStrategy implements RolloverStrategy {
+private static final Logger LOGGER =
LoggerFactory.getLogger(DeleteOldRolloverStrategy.class);
+private static final DateTimeFormatter DATE_FORMATTER =
DateTimeFormatter.ofPattern("-MM-dd").withZone(ZoneOffset.UTC);
Review Comment:
Is there a reason for using `UTC` in this case? It seems like it would be
more intuitive to use `LocalDateTime`, matching the system time.
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/embedded/EmbeddedClient.java:
##
@@ -0,0 +1,151 @@
+/*
+ * 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.questdb.embedded;
+
+import io.questdb.cairo.CairoEngine;
+import io.questdb.cairo.CairoError;
+import io.questdb.cairo.TableToken;
+import io.questdb.cairo.TableWriter;
+import io.questdb.cairo.sql.RecordCursor;
+import io.questdb.cairo.sql.RecordCursorFactory;
+import io.questdb.griffin.CompiledQuery;
+import io.questdb.griffin.SqlCompiler;
+import io.questdb.griffin.SqlCompilerFactoryImpl;
+import io.questdb.griffin.SqlException;
+import io.questdb.griffin.SqlExecutionContext;
+import io.questdb.mp.SCSequence;
+import io.questdb.mp.TimeoutBlockingWaitStrategy;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.InsertRowDataSource;
+import org.apache.nifi.questdb.QueryResultProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1447199207
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/embedded/LockedClient.java:
##
@@ -0,0 +1,105 @@
+/*
+ * 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.questdb.embedded;
+
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.InsertRowDataSource;
+import org.apache.nifi.questdb.QueryResultProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+
+final class LockedClient implements Client {
Review Comment:
This lock server different purposes than the database's internal lock. Due
to the embedded nature of the database, the maintenance of it is a
responsibility taken care by the `EmbeddedDatabaseManager`. This comes with
time periods where the database is tampered with (kind of under maintenance)
when we do not want to interact with the database. This is prevented and
encapsulated by the `LockedClient`. Together with the retry, this should make
shorter outages (recreating database) unnoticable for clients.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1447192926
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManagerContext.java:
##
@@ -0,0 +1,39 @@
+/*
+ * 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.questdb.embedded;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+interface EmbeddedDatabaseManagerContext {
+String getPersistLocation();
+Path getPersistLocationAsPath();
+File getPersistLocationAsFile();
+
+String getBackupLocation();
+Path getBackupLocationAsPath();
+File getBackupLocationAsFile();
Review Comment:
It was for convinience but I removed that
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1447192609
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/embedded/EmbeddedDatabaseManager.java:
##
@@ -0,0 +1,297 @@
+/*
+ * 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.questdb.embedded;
+
+import io.questdb.cairo.CairoConfiguration;
+import io.questdb.cairo.CairoEngine;
+import io.questdb.cairo.DefaultCairoConfiguration;
+import io.questdb.cairo.TableToken;
+import io.questdb.cairo.sql.TableRecordMetadata;
+import org.apache.commons.lang3.concurrent.BasicThreadFactory;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.DatabaseManager;
+import org.apache.nifi.util.file.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.stream.Collectors;
+
+final class EmbeddedDatabaseManager implements DatabaseManager {
+private static final Logger LOGGER =
LoggerFactory.getLogger(EmbeddedDatabaseManager.class);
+
+private final String id = UUID.randomUUID().toString();
+private final AtomicReference state = new
AtomicReference(EmbeddedDatabaseManagerStatus.UNINITIALIZED);
+private final ReadWriteLock databaseStructureLock = new
ReentrantReadWriteLock();
+private final EmbeddedDatabaseManagerContext context;
+private final AtomicReference engine = new
AtomicReference<>();
+private final List> scheduledFutures = new
ArrayList<>();
+private final ScheduledExecutorService scheduledExecutorService = Executors
+.newScheduledThreadPool(2, new
BasicThreadFactory.Builder().namingPattern("EmbeddedQuestDbManagerWorker-" + id
+ "-%d").build());
+
+EmbeddedDatabaseManager(final EmbeddedDatabaseManagerContext context) {
+this.context = context;
+}
+
+@Override
+public void init() {
+if (state.get() != EmbeddedDatabaseManagerStatus.UNINITIALIZED) {
+throw new IllegalStateException("Manager is already initialized!");
+}
+
+ensureDatabaseIsReady();
+startRollover();
+}
+
+private void ensureDatabaseIsReady() {
+boolean successful = false;
+
+try {
+databaseStructureLock.writeLock().lock();
+state.set(EmbeddedDatabaseManagerStatus.REPAIRING);
+
+try {
+ensurePersistLocationIsAccessible();
+ensureConnectionEstablished();
+ensureTablesAreInPlaceAndHealthy();
+successful = true;
+} catch (final CorruptedDatabaseException e) {
+boolean couldMoveOldToBackup = false;
+
+try {
+LOGGER.error("Database is corrupted. Recreation is
triggered.", e);
+final File backupFolder = new
File(context.getBackupLocationAsFile(), "backup_" + System.currentTimeMillis());
+
FileUtils.ensureDirectoryExistAndCanAccess(context.getBackupLocationAsFile());
+Files.move(context.getPersistLocationAsPath(),
backupFolder.toPath());
+couldMoveOldToBackup = true;
+} catch (IOException ex) {
+LOGGER.error("Could not create backup", ex);
+}
+
+if (!couldMoveOldToBackup) {
+try {
+
FileUtils.deleteFile(context.getPersistLocationAsFile(), true);
+couldMoveOldToBackup = true;
+} catch (IOException ex) {
+LOGGER.error("Could not clean up corrupted database",
ex);
+
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1447186671
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/embedded/EmbeddedClient.java:
##
@@ -0,0 +1,151 @@
+/*
+ * 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.questdb.embedded;
+
+import io.questdb.cairo.CairoEngine;
+import io.questdb.cairo.CairoError;
+import io.questdb.cairo.TableToken;
+import io.questdb.cairo.TableWriter;
+import io.questdb.cairo.sql.RecordCursor;
+import io.questdb.cairo.sql.RecordCursorFactory;
+import io.questdb.griffin.CompiledQuery;
+import io.questdb.griffin.SqlCompiler;
+import io.questdb.griffin.SqlCompilerFactoryImpl;
+import io.questdb.griffin.SqlException;
+import io.questdb.griffin.SqlExecutionContext;
+import io.questdb.mp.SCSequence;
+import io.questdb.mp.TimeoutBlockingWaitStrategy;
+import org.apache.nifi.questdb.Client;
+import org.apache.nifi.questdb.DatabaseException;
+import org.apache.nifi.questdb.InsertRowDataSource;
+import org.apache.nifi.questdb.QueryResultProcessor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
+
+final class EmbeddedClient implements Client {
+private final static Logger LOGGER =
LoggerFactory.getLogger(EmbeddedClient.class);
+
+private final Supplier engine;
+private final AtomicBoolean disconnected = new AtomicBoolean(false);
+
+EmbeddedClient(final Supplier engine) {
+this.engine = engine;
+}
+
+@Override
+public void execute(final String query) throws DatabaseException {
+checkConnectionState();
+
+try (final SqlCompiler compiler = getCompiler()) {
+final CompiledQuery compile = compiler.compile(query,
getSqlExecutionContext());
+compile.execute(new SCSequence(new TimeoutBlockingWaitStrategy(5,
TimeUnit.SECONDS)));
+} catch (final SqlException | CairoError e) {
+throw new DatabaseException(e);
+}
+}
+
+@Override
+public void insert(
+final String tableName,
+final InsertRowDataSource rowDataSource
+) throws DatabaseException {
+checkConnectionState();
+
+if (!rowDataSource.hasNextToInsert()) {
+LOGGER.debug("No rows to insert into {}", tableName);
+return;
+}
+
+final TableToken tableToken =
engine.get().getTableTokenIfExists(tableName);
+
+if (tableToken == null) {
+throw new DatabaseException(String.format("Table Token for table
[%s] not found", tableName));
+}
+
+try (
+final TableWriter tableWriter = engine.get().getWriter(tableToken,
"adding rows")
+) {
+final TableWriterBasedInsertRowContext context = new
TableWriterBasedInsertRowContext(tableWriter);
+
+while (rowDataSource.hasNextToInsert()) {
+context.addRow(rowDataSource);
+}
+
+LOGGER.debug("Committing {} rows", tableWriter.getRowCount());
+tableWriter.commit();
+} catch (final Exception | CairoError e) {
+// CairoError might be thrown in extreme cases, for example when
no space left on the disk
+throw new DatabaseException(e);
+} finally {
+engine.get().releaseInactive();
+}
+}
+
+@Override
+public T query(final String query, final QueryResultProcessor
rowProcessor) throws DatabaseException {
+checkConnectionState();
+
+final CompiledQuery compiledQuery;
+
+try (final SqlCompiler compiler = getCompiler()) {
+compiledQuery = compiler.compile(query, getSqlExecutionContext());
+} catch (final SqlException | CairoError e) {
+throw new DatabaseException(e);
+}
+
+try (
+final RecordCursorFactory factory =
compiledQuery.getRecordCursorFactory();
+final RecordCursor cursor =
factory.getCursor(getSqlExecutionContext());
+) {
+final CursorBasedQueryRowContext rowContext = new
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1447185052
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb/src/main/java/org/apache/nifi/questdb/DatabaseManager.java:
##
@@ -14,25 +14,25 @@
* 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.nifi.controller.status.NodeStatus;
-import org.apache.nifi.controller.status.history.StatusHistory;
-
-import java.time.Instant;
+package org.apache.nifi.questdb;
/**
- * Readable status storage for the node status entries.
+ * Provides access to database via distributing clients. Also responsible to
ensure the health of the database connection
+ * and database if possible.
*/
-public interface NodeStatusStorage extends StatusStorage {
+public interface DatabaseManager {
+/**
+ * @return A client to execute queries against the managed database
instance.
+ */
+Client acquireClient();
+
+/**
+ * Starts maintenance of the database. Necessary initialization step for
proper use.
+ */
+void init();
/**
- * Returns with the status history of the node for the specified time
range.
- *
- * @param start Start date of the history, inclusive.
- * @param end End date of the history, inclusive.
- *
- * @return Status history.
+ * Finishes maintenance of the database. After calling, manager does not
guarantee any connection with the database.
*/
-StatusHistory read(Instant start, Instant end);
+void close();
Review Comment:
Yeah, that is true. I added it but kept the original method signiture
without the exception as well. Currently we do not build on the "closability"
and where it is considered as `DatabaseManager` we do not expect exception when
closing.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1447182808
##
nifi-commons/nifi-utils/src/test/java/org/apache/nifi/retry/ExceptionExcludingRetryConditionTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.retry;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import javax.naming.OperationNotSupportedException;
+import java.io.IOException;
+import java.util.Arrays;
+
+class ExceptionExcludingRetryConditionTest {
+final ExceptionExcludingRetryCondition testSubject = getTestSubject();
Review Comment:
I think in most cases (especially when the test scenario is more
complicated) having `testSubject` as name leads the eye making it easier and
faster to understand the test. If you do not insist, I would prefer to keep it
as it is.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1447178610
##
nifi-commons/nifi-utils/src/main/java/org/apache/nifi/retry/NoReturnCallable.java:
##
@@ -14,25 +14,16 @@
* 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.nifi.controller.status.history.GarbageCollectionHistory;
-import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
-
-import java.time.Instant;
+package org.apache.nifi.retry;
/**
- * Readable status storage for garbage collection status entries.
+ * Represents a portion of callable code without expected return value.
*/
-public interface GarbageCollectionStatusStorage extends
StatusStorage {
+@FunctionalInterface
+public interface NoReturnCallable {
Review Comment:
Good idea, I tried and it looks to me that the sprint-retry can be
configured to provide the same behaviour.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1435330854
##
nifi-commons/nifi-utils/src/test/java/org/apache/nifi/retry/ExceptionExcludingRetryConditionTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.retry;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import javax.naming.OperationNotSupportedException;
+import java.io.IOException;
+import java.util.Arrays;
+
+class ExceptionExcludingRetryConditionTest {
+final ExceptionExcludingRetryCondition testSubject = getTestSubject();
+
+@Test
+public void testAllowIfNoException() {
+final MutableRetryExecutionContext context = new
MutableRetryExecutionContext();
+Assertions.assertTrue(testSubject.allowNextAttempt(context));
Review Comment:
As a general rule, it is best to use static imports for assert and mock
methods to helpful with readability of tests.
##
nifi-commons/nifi-utils/src/test/java/org/apache/nifi/retry/SynchronousRetryTemplateTest.java:
##
@@ -0,0 +1,224 @@
+/*
+ * 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.retry;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.LinkedList;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.function.BiConsumer;
+
+public class SynchronousRetryTemplateTest {
+public static final int NUMBER_OF_RETRIES = 2;
+public static final Callable FALLBACK_ACTION = () -> "fallback";
+public static final Callable FALLBACK_ACTION_FAILING = () -> {
Review Comment:
Are these values used in other test classes? I may have missed the
references, otherwise it would be helpful to lower the visibility.
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/QuestDbStatusHistoryStorage.java:
##
@@ -0,0 +1,181 @@
+/*
+ * 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 org.apache.nifi.controller.status.ConnectionStatus;
+import org.apache.nifi.controller.status.NodeStatus;
+import org.apache.nifi.controller.status.ProcessGroupStatus;
+import org.apache.nifi.controller.status.ProcessorStatus;
+import org.apache.nifi.controller.status.RemoteProcessGroupStatus;
+import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
+import org.apache.nifi.controller.status.history.StandardMetricDescriptor;
+import org.apache.nifi.controller.status.history.StandardStatusSnapshot;
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1435328555
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/EmbeddedQuestDbStatusHistoryRepositoryDefinitions.java:
##
@@ -0,0 +1,343 @@
+/*
+ * 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 org.apache.nifi.controller.status.ConnectionStatus;
+import org.apache.nifi.controller.status.NodeStatus;
+import org.apache.nifi.controller.status.ProcessGroupStatus;
+import org.apache.nifi.controller.status.ProcessorStatus;
+import org.apache.nifi.controller.status.RemoteProcessGroupStatus;
+import org.apache.nifi.controller.status.history.ConnectionStatusDescriptor;
+import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
+import org.apache.nifi.controller.status.history.MetricDescriptor;
+import org.apache.nifi.controller.status.history.NodeStatusDescriptor;
+import org.apache.nifi.controller.status.history.ProcessGroupStatusDescriptor;
+import org.apache.nifi.controller.status.history.ProcessorStatusDescriptor;
+import
org.apache.nifi.controller.status.history.RemoteProcessGroupStatusDescriptor;
+import org.apache.nifi.controller.status.history.StandardMetricDescriptor;
+import org.apache.nifi.controller.status.history.StandardStatusSnapshot;
+import org.apache.nifi.questdb.InsertRowDataSource;
+import org.apache.nifi.questdb.QueryResultProcessor;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.mapping.RequestMappingBuilder;
+
+import java.time.ZoneId;
+import java.time.format.DateTimeFormatter;
+import java.util.Collection;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+final class EmbeddedQuestDbStatusHistoryRepositoryDefinitions {
+/**
+ * Date format expected by the storage.
+ */
+static final String CAPTURE_DATE_FORMAT = "-MM-dd:HH:mm:ss Z";
+
+/**
+ * Date formatter for the database fields.
+ */
+static final DateTimeFormatter DATE_FORMATTER =
DateTimeFormatter.ofPattern(CAPTURE_DATE_FORMAT).withZone(ZoneId.systemDefault());
+
+// General component
+
+static final String COMPONENT_STATUS_QUERY =
+"SELECT * FROM %s " +
+"WHERE componentId = '%s' " +
+"AND capturedAt > to_timestamp('%s', '" + CAPTURE_DATE_FORMAT + "') " +
+"AND capturedAt < to_timestamp('%s', '" + CAPTURE_DATE_FORMAT + "') " +
+"ORDER BY capturedAt ASC";
+
+// Connection
+
+static final String TABLE_NAME_CONNECTION_STATUS = "connectionStatus";
+
+static final String CREATE_CONNECTION_STATUS =
+"CREATE TABLE " + TABLE_NAME_CONNECTION_STATUS + " (" +
+"capturedAt TIMESTAMP," +
+"componentId SYMBOL capacity 2000 nocache index capacity 1500," +
+"inputBytes LONG," +
+"inputCount LONG," +
+"outputBytes LONG," +
+"outputCount LONG," +
+"queuedBytes LONG," +
+"queuedCount LONG," +
+"totalQueuedDuration LONG," +
+"maxQueuedDuration LONG," +
+"averageQueuedDuration LONG" +
+") TIMESTAMP(capturedAt) PARTITION BY DAY";
+
+private static final Map>
CONNECTION_METRICS = new HashMap<>() {{
+put(2, ConnectionStatusDescriptor.INPUT_BYTES.getDescriptor());
+put(3, ConnectionStatusDescriptor.INPUT_COUNT.getDescriptor());
+put(4, ConnectionStatusDescriptor.OUTPUT_BYTES.getDescriptor());
+put(5, ConnectionStatusDescriptor.OUTPUT_COUNT.getDescriptor());
+put(6, ConnectionStatusDescriptor.QUEUED_BYTES.getDescriptor());
+put(7, ConnectionStatusDescriptor.QUEUED_COUNT.getDescriptor());
+put(8,
ConnectionStatusDescriptor.TOTAL_QUEUED_DURATION.getDescriptor());
+put(9, ConnectionStatusDescriptor.MAX_QUEUED_DURATION.getDescriptor());
+put(10,
ConnectionStatusDescriptor.AVERAGE_QUEUED_DURATION.getDescriptor());
+}};
Review Comment:
Although not all of the bullet points may be applicable,
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1435326266
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/CapturedStatus.java:
##
@@ -0,0 +1,51 @@
+/*
+ * 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 java.time.Instant;
+import java.util.Objects;
+
+final class CapturedStatus {
+private final T status;
+private final Instant capturedAt;
Review Comment:
Taking the `nifi-client-dto` as an example, timestamp properties in those
classes due not have the `At` suffix, and include properties such as `created`
and `lastUpdated`. Following that convention, I recommend removing all
occurrences of the `At` suffix from property names.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1435323896
##
nifi-commons/nifi-utils/src/main/java/org/apache/nifi/retry/NoReturnCallable.java:
##
@@ -14,25 +14,16 @@
* 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.nifi.controller.status.history.GarbageCollectionHistory;
-import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
-
-import java.time.Instant;
+package org.apache.nifi.retry;
/**
- * Readable status storage for garbage collection status entries.
+ * Represents a portion of callable code without expected return value.
*/
-public interface GarbageCollectionStatusStorage extends
StatusStorage {
+@FunctionalInterface
+public interface NoReturnCallable {
Review Comment:
Thanks for the additional background. With that in mind, it seems like
having the custom `Callable` as a nested interface on the RetryTemplate would
be a better approach.
Did you consider using
[spring-retry](https://www.baeldung.com/spring-retry#retrytemplate) instead?
Since the abstraction applies to QuestDB, it seems worth considering as it has
a minimal number of dependencies.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1424892774
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/StatusHistoryStorage.java:
##
@@ -0,0 +1,50 @@
+/*
+ * 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 org.apache.nifi.controller.status.ConnectionStatus;
+import org.apache.nifi.controller.status.NodeStatus;
+import org.apache.nifi.controller.status.ProcessGroupStatus;
+import org.apache.nifi.controller.status.ProcessorStatus;
+import org.apache.nifi.controller.status.RemoteProcessGroupStatus;
+import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
+import org.apache.nifi.controller.status.history.StatusSnapshot;
+
+import java.util.Collection;
+import java.util.Date;
+import java.util.List;
+
+interface StatusHistoryStorage {
+
+default void init() {};
+default void close() {};
+
+List getConnectionSnapshots(final String componentId,
final Date start, final Date end);
Review Comment:
It is called from `public StatusHistory getConnectionStatusHistory(final
String connectionId, final Date start, final Date end, final int
preferredDataPoints)` which is defined by the `StatusHistoryRepository`
interface. I made it to the responsiblity of the underlaying storage to convert
the dates as it needs.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1424889502
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/EmbeddedQuestDbStatusHistoryRepositoryDefinitions.java:
##
@@ -0,0 +1,343 @@
+/*
+ * 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 org.apache.nifi.controller.status.ConnectionStatus;
+import org.apache.nifi.controller.status.NodeStatus;
+import org.apache.nifi.controller.status.ProcessGroupStatus;
+import org.apache.nifi.controller.status.ProcessorStatus;
+import org.apache.nifi.controller.status.RemoteProcessGroupStatus;
+import org.apache.nifi.controller.status.history.ConnectionStatusDescriptor;
+import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
+import org.apache.nifi.controller.status.history.MetricDescriptor;
+import org.apache.nifi.controller.status.history.NodeStatusDescriptor;
+import org.apache.nifi.controller.status.history.ProcessGroupStatusDescriptor;
+import org.apache.nifi.controller.status.history.ProcessorStatusDescriptor;
+import
org.apache.nifi.controller.status.history.RemoteProcessGroupStatusDescriptor;
+import org.apache.nifi.controller.status.history.StandardMetricDescriptor;
+import org.apache.nifi.controller.status.history.StandardStatusSnapshot;
+import org.apache.nifi.questdb.InsertRowDataSource;
+import org.apache.nifi.questdb.QueryResultProcessor;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.mapping.RequestMappingBuilder;
+
+import java.time.ZoneId;
+import java.time.format.DateTimeFormatter;
+import java.util.Collection;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+final class EmbeddedQuestDbStatusHistoryRepositoryDefinitions {
+/**
+ * Date format expected by the storage.
+ */
+static final String CAPTURE_DATE_FORMAT = "-MM-dd:HH:mm:ss Z";
+
+/**
+ * Date formatter for the database fields.
+ */
+static final DateTimeFormatter DATE_FORMATTER =
DateTimeFormatter.ofPattern(CAPTURE_DATE_FORMAT).withZone(ZoneId.systemDefault());
+
+// General component
+
+static final String COMPONENT_STATUS_QUERY =
+"SELECT * FROM %s " +
+"WHERE componentId = '%s' " +
+"AND capturedAt > to_timestamp('%s', '" + CAPTURE_DATE_FORMAT + "') " +
+"AND capturedAt < to_timestamp('%s', '" + CAPTURE_DATE_FORMAT + "') " +
+"ORDER BY capturedAt ASC";
+
+// Connection
+
+static final String TABLE_NAME_CONNECTION_STATUS = "connectionStatus";
+
+static final String CREATE_CONNECTION_STATUS =
+"CREATE TABLE " + TABLE_NAME_CONNECTION_STATUS + " (" +
+"capturedAt TIMESTAMP," +
+"componentId SYMBOL capacity 2000 nocache index capacity 1500," +
+"inputBytes LONG," +
+"inputCount LONG," +
+"outputBytes LONG," +
+"outputCount LONG," +
+"queuedBytes LONG," +
+"queuedCount LONG," +
+"totalQueuedDuration LONG," +
+"maxQueuedDuration LONG," +
+"averageQueuedDuration LONG" +
+") TIMESTAMP(capturedAt) PARTITION BY DAY";
+
+private static final Map>
CONNECTION_METRICS = new HashMap<>() {{
+put(2, ConnectionStatusDescriptor.INPUT_BYTES.getDescriptor());
+put(3, ConnectionStatusDescriptor.INPUT_COUNT.getDescriptor());
+put(4, ConnectionStatusDescriptor.OUTPUT_BYTES.getDescriptor());
+put(5, ConnectionStatusDescriptor.OUTPUT_COUNT.getDescriptor());
+put(6, ConnectionStatusDescriptor.QUEUED_BYTES.getDescriptor());
+put(7, ConnectionStatusDescriptor.QUEUED_COUNT.getDescriptor());
+put(8,
ConnectionStatusDescriptor.TOTAL_QUEUED_DURATION.getDescriptor());
+put(9, ConnectionStatusDescriptor.MAX_QUEUED_DURATION.getDescriptor());
+put(10,
ConnectionStatusDescriptor.AVERAGE_QUEUED_DURATION.getDescriptor());
+}};
Review Comment:
The order is important but not without gaps, so the `Map.of()`
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1424884706
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/EmbeddedQuestDbStatusHistoryRepositoryDefinitions.java:
##
@@ -0,0 +1,343 @@
+/*
+ * 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 org.apache.nifi.controller.status.ConnectionStatus;
+import org.apache.nifi.controller.status.NodeStatus;
+import org.apache.nifi.controller.status.ProcessGroupStatus;
+import org.apache.nifi.controller.status.ProcessorStatus;
+import org.apache.nifi.controller.status.RemoteProcessGroupStatus;
+import org.apache.nifi.controller.status.history.ConnectionStatusDescriptor;
+import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
+import org.apache.nifi.controller.status.history.MetricDescriptor;
+import org.apache.nifi.controller.status.history.NodeStatusDescriptor;
+import org.apache.nifi.controller.status.history.ProcessGroupStatusDescriptor;
+import org.apache.nifi.controller.status.history.ProcessorStatusDescriptor;
+import
org.apache.nifi.controller.status.history.RemoteProcessGroupStatusDescriptor;
+import org.apache.nifi.controller.status.history.StandardMetricDescriptor;
+import org.apache.nifi.controller.status.history.StandardStatusSnapshot;
+import org.apache.nifi.questdb.InsertRowDataSource;
+import org.apache.nifi.questdb.QueryResultProcessor;
+import org.apache.nifi.questdb.mapping.RequestMapping;
+import org.apache.nifi.questdb.mapping.RequestMappingBuilder;
+
+import java.time.ZoneId;
+import java.time.format.DateTimeFormatter;
+import java.util.Collection;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+final class EmbeddedQuestDbStatusHistoryRepositoryDefinitions {
+/**
+ * Date format expected by the storage.
+ */
+static final String CAPTURE_DATE_FORMAT = "-MM-dd:HH:mm:ss Z";
Review Comment:
I do not have the details in hand, as this is from the original
implementation, but I think at the time of adding QuestDB this was either the
supported or properly working format.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1424882496
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/CapturedStatus.java:
##
@@ -0,0 +1,51 @@
+/*
+ * 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 java.time.Instant;
+import java.util.Objects;
+
+final class CapturedStatus {
+private final T status;
+private final Instant capturedAt;
Review Comment:
The implementation uses the term "captured at" from the database tables over
all. In some places I think the "at" suffix fastens understanding that it is a
time based attribute. So I can change it but in that case I think we should
change it everywhere. What are your thoughts?
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152: URL: https://github.com/apache/nifi/pull/8152#discussion_r1424878344 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-status-history-shared/pom.xml: ## @@ -30,5 +30,9 @@ org.apache.nifi nifi-framework-api + +org.apache.commons +commons-lang3 Review Comment: You are all right, it is not necessary any more. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152: URL: https://github.com/apache/nifi/pull/8152#discussion_r1424876658 ## nifi-docs/src/main/asciidoc/administration-guide.adoc: ## @@ -3538,15 +3538,19 @@ of 576. Persistent repository -If the value of the property `nifi.components.status.repository.implementation` is `EmbeddedQuestDbStatusHistoryRepository`, the -status history data will be stored to the disk in a persistent manner. Data will be kept between restarts. +If the value of the property `nifi.components.status.repository.implementation` is `org.apache.nifi.controller.status.history.questdb.EmbeddedQuestDbStatusHistoryRepository`, the +status history data will be stored to the disk in a persistent manner. Data will be kept between restarts. In order to use persistent repository, the QuestDB NAR must be re-built with the `include-questdb` profiles enabled. | |*Property*|*Description* |`nifi.status.repository.questdb.persist.node.days`|The number of days the node status data (such as Repository disk space free, garbage collection information, etc.) will be kept. The default values is `14`. |`nifi.status.repository.questdb.persist.component.days`|The number of days the component status data (i.e., stats for each Processor, Connection, etc.) will be kept. The default value is `3`. |`nifi.status.repository.questdb.persist.location`|The location of the persistent Status History Repository. The default value is `./status_repository`. +|`nifi.status.repository.questdb.persist.location.backup`|The location of the database backup in case the database is being corrupted and recreated. The default value is `./status_repository_backup`. +|`nifi.status.repository.questdb.persist.batchsize`|The QuestDb based status history repository persists the collected status information in batches. The batch size determines the maximum number of persisted status records at a given time. The default value is `1000`. Review Comment: In most situations I think the default settings should work well and this is the reason they were not exposed in the original implementation. In cases where there are high number of processor, or the node is under high load I find it useful to give the opportunity to fine tune. What I am thinking about for example is in case of 10-30k of processors might use bigger batch size for persisting. These can be more extreme cases but I find it useful to have the opportunity for affecting on the behaviour. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1424874235
##
nifi-commons/nifi-utils/src/main/java/org/apache/nifi/retry/NoReturnCallable.java:
##
@@ -14,25 +14,16 @@
* 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.nifi.controller.status.history.GarbageCollectionHistory;
-import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
-
-import java.time.Instant;
+package org.apache.nifi.retry;
/**
- * Readable status storage for garbage collection status entries.
+ * Represents a portion of callable code without expected return value.
*/
-public interface GarbageCollectionStatusStorage extends
StatusStorage {
+@FunctionalInterface
+public interface NoReturnCallable {
Review Comment:
I think, from Java Gerenics standpoint, `Void` could work out. Where it
fails however is the way of intended usage. The `RetryTemplate` is primarily
designed to be called not with explicit implementations of `Callable` or
`NoReturnCallable` for that sake, but to give lambda expressions as arguments,
like the following (from `SynchronousRetryTemplateTest`):
`testSubject.execute(() -> action.getValue("okay"), FALLBACK_ACTION);`
Creating an expression with `Void` return value to fit this intended use
look to be cumbersome. So by adding `NoReturnCallable` as a functional
interface, which internally wraps these "void return" or "no return" lambdas
into a 'Callable' will solve the question of easy use. With this, a no return
expression can be as simple as:
`testSubject.executeWithoutValue(() -> action.consumeValue("okay"));`
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8152: URL: https://github.com/apache/nifi/pull/8152#issuecomment-1853298122 > Thanks for creating the new pull request and making the adjustments @simonbence. I have yet not review the entire implementation, but I noted a few recommendations. > > Having this in a separate module and avoiding changes to the nifi-api is helpful. I am not sure about the general reuse of the retry interfaces, so I'm not sure whether it warrants adding them to `nifi-utils`. Perhaps they would be better in a separate `nifi-retry-utils` under `nifi-commons`, or just kept as part of the QuestDB module in a separate package until it gets to the point where they could be reused. I hope this PR will be more fit to go on with :) As of retry: even if it is currently used only by the QuestDB module, I would consider it as a general util or tool, just like other parts of the given module, like for example the `InputStream` implementations. The concept is simple and might be applied in many place and I think we would benefit from having one extracted and tested code piece do this work if necessary. So I would not think moving it to the QuestDB module would be the right way but if you insist, I can separate it to a distinct module under `nifi-commons` (however I think that would be a bit of an exaggeration). Please share your thoughts, thanks! -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8152:
URL: https://github.com/apache/nifi/pull/8152#discussion_r1424815114
##
nifi-commons/nifi-utils/src/main/java/org/apache/nifi/retry/NoReturnCallable.java:
##
@@ -14,25 +14,16 @@
* 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.nifi.controller.status.history.GarbageCollectionHistory;
-import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
-
-import java.time.Instant;
+package org.apache.nifi.retry;
/**
- * Readable status storage for garbage collection status entries.
+ * Represents a portion of callable code without expected return value.
*/
-public interface GarbageCollectionStatusStorage extends
StatusStorage {
+@FunctionalInterface
+public interface NoReturnCallable {
Review Comment:
There seems like a very generic interface. Is it necessary as opposed to the
standard `Runnable`, or `Callable` with `Void` return?
##
nifi-docs/src/main/asciidoc/administration-guide.adoc:
##
@@ -3538,15 +3538,19 @@ of 576.
Persistent repository
-If the value of the property
`nifi.components.status.repository.implementation` is
`EmbeddedQuestDbStatusHistoryRepository`, the
-status history data will be stored to the disk in a persistent manner. Data
will be kept between restarts.
+If the value of the property
`nifi.components.status.repository.implementation` is
`org.apache.nifi.controller.status.history.questdb.EmbeddedQuestDbStatusHistoryRepository`,
the
+status history data will be stored to the disk in a persistent manner. Data
will be kept between restarts. In order to use persistent repository, the
QuestDB NAR must be re-built with the `include-questdb` profiles enabled.
|
|*Property*|*Description*
|`nifi.status.repository.questdb.persist.node.days`|The number of days the
node status data (such as Repository disk space free, garbage collection
information, etc.) will be kept. The default values
is `14`.
|`nifi.status.repository.questdb.persist.component.days`|The number of days
the component status data (i.e., stats for each Processor, Connection, etc.)
will be kept. The default value is `3`.
|`nifi.status.repository.questdb.persist.location`|The location of the
persistent Status History Repository. The default value is
`./status_repository`.
+|`nifi.status.repository.questdb.persist.location.backup`|The location of the
database backup in case the database is being corrupted and recreated. The
default value is `./status_repository_backup`.
+|`nifi.status.repository.questdb.persist.batchsize`|The QuestDb based status
history repository persists the collected status information in batches. The
batch size determines the maximum number of persisted status records at a given
time. The default value is `1000`.
Review Comment:
How likely is it that this will need to be changed? Is it a case where
having an internal default is sufficient? I suppose it depends on the volume of
data, but if it does not need to be changed, it may be better to keep it as an
internal setting versus a public property.
##
nifi-nar-bundles/nifi-questdb-bundle/nifi-questdb-status-history/src/main/java/org/apache/nifi/controller/status/history/questdb/EmbeddedQuestDbStatusHistoryRepositoryDefinitions.java:
##
@@ -0,0 +1,343 @@
+/*
+ * 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 org.apache.nifi.controller.status.ConnectionStatus;
+import org.apache.nifi.controller.status.NodeStatus;
+import org.apache.nifi.controller.status.ProcessGroupStatus;
+import org.apache.nifi.controller.status.ProcessorStatus;
+import org.apache.nifi.controller.status.RemoteProcessGroupStatus;
+import org.apache.nifi.controller.status.history.ConnectionStatusDescriptor;
+import org.apache.nifi.controller.status.history.GarbageCollectionStatus;
+import org.apache.nifi.controller.status.history.MetricDescriptor;
+import org.apache.nifi.controller.status.history.NodeStatusDescriptor;
+import
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence closed pull request #8123: NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository URL: https://github.com/apache/nifi/pull/8123 -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8123: URL: https://github.com/apache/nifi/pull/8123#issuecomment-1851737367 As the changes discussed on the ticket and were resulted by NIFI-12492 would make this PR hard to follow, I close it and will open a new one -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8123:
URL: https://github.com/apache/nifi/pull/8123#discussion_r1419100364
##
nifi-api/src/main/java/org/apache/nifi/controller/status/ConnectionStatus.java:
##
@@ -22,6 +22,7 @@
/**
*/
public class ConnectionStatus implements Cloneable {
+private long createdAtInMs;
Review Comment:
Thanks for the reply @simonbence.
Reviewing the usage, in particular reference to `AbstractEventAccess`, the
`created` timestamp here appears somewhat ambiguous. The usage in
`AbstractEventAccess` appears to populate this value based on the current
system time. From that perspective, `created` appears to mean the time at which
the Status object was created. However, the rest of the properties in the
Status object refer to the referenced object itself, such as the Connection or
Process Group. From that perspective, I would have expected a created timestamp
to mean the time at which the component was created. This highlights the
problem with adding this property to the NiFi API.
Although it might be possible to clarify the ambiguity with a different
name, it still falls into a different category than all of the other properties.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8123:
URL: https://github.com/apache/nifi/pull/8123#discussion_r1419082031
##
nifi-api/src/main/java/org/apache/nifi/controller/status/ConnectionStatus.java:
##
@@ -22,6 +22,7 @@
/**
*/
public class ConnectionStatus implements Cloneable {
+private long createdAtInMs;
Review Comment:
The implementation was the trigger for the change but not the sole reason.
The idea was -based on how we use these- that the state snapshot and the time
of relevance travels together in many times which makes it suspicious that they
should be boundled together. I will not touch until you make your rounds, let's
discuss after that how you feel about the approach.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8123:
URL: https://github.com/apache/nifi/pull/8123#discussion_r1417672045
##
nifi-api/src/main/java/org/apache/nifi/controller/status/NodeStatus.java:
##
@@ -35,7 +35,6 @@ public class NodeStatus implements Cloneable {
private double processorLoadAverage;
private long totalThreads;
-private long eventDrivenThreads;
Review Comment:
This change is a helpful cleanup, which would be useful to handle
separately. As this particular pull request may take some time to review, would
you consider separating out this removal to its own pull request?
##
nifi-commons/nifi-questdb/pom.xml:
##
@@ -0,0 +1,71 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
https://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache.nifi
+nifi-commons
+2.0.0-SNAPSHOT
+
+nifi-questdb
+
+
+
+org.apache.nifi
+nifi-utils
+2.0.0-SNAPSHOT
+
+
+
+org.apache.commons
+commons-lang3
+3.13.0
+
+
+commons-io
+commons-io
+2.13.0
+
+
+org.slf4j
+slf4j-api
+2.0.9
+
+
+org.questdb
+questdb
+7.2
+
+
+
+org.junit.jupiter
+junit-jupiter-engine
+5.9.2
+test
+
+
+org.mockito
+mockito-core
+5.6.0
+test
+
+
+org.mockito
+mockito-junit-jupiter
+5.6.0
+
Review Comment:
With the exception of `questdb`, all of these dependency versions are
managed at the root Maven configuration, so the specific version numbers should
be removed.
##
nifi-api/src/main/java/org/apache/nifi/controller/status/ConnectionStatus.java:
##
@@ -22,6 +22,7 @@
/**
*/
public class ConnectionStatus implements Cloneable {
+private long createdAtInMs;
Review Comment:
Although it might require some additional replication in the QuestDB
implementation itself, it is important to avoid introducing specific concepts
to the NiFi API that do not have general applicability. I would have to take a
closer look at how this is used, but in general, I believe any changes to the
nifi-api should be avoided for the purpose of this implementation.
##
nifi-commons/nifi-questdb/src/test/java/org/apache/nifi/questdb/QuestDbTestUtil.java:
##
@@ -0,0 +1,88 @@
+/*
+ * 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.questdb;
+
+import java.io.File;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.UUID;
+
+public class QuestDbTestUtil {
+static final File TEST_DB_PATH = new File("src/test/resources/testdb");
Review Comment:
The JUnit 5 `@TempDir` annotation is preferable to hard-coded paths that
reference source directories, since that annotation provides lifecycle
management for temporary directories.
##
nifi-commons/nifi-questdb/src/test/java/org/apache/nifi/questdb/CompositeClientTest.java:
##
@@ -0,0 +1,218 @@
+/*
+ * 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
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8123: URL: https://github.com/apache/nifi/pull/8123#issuecomment-1842633320 Thansk @joewitt ! I answered on the JIRA as well. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on code in PR #8123:
URL: https://github.com/apache/nifi/pull/8123#discussion_r1417093557
##
nifi-api/src/main/java/org/apache/nifi/controller/status/ConnectionStatus.java:
##
@@ -22,6 +22,7 @@
/**
*/
public class ConnectionStatus implements Cloneable {
+private long createdAtInMs;
Review Comment:
Based on most usage I would think a time related information would make
sense within status (at what time the status snapshot is relevant). I am not
sure this is true for all usages but the current setup on main (these two
coupled information travels together) does not seem straightforward to me. I
cannot rule out however a possible third solution which complies with both
things.
Instant is a good idea. I think I went down this road because some already
contained creation time in this flavor but the `Instant` really makes more
sense (especially that we convert it to instant many places already)
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
simonbence commented on PR #8123: URL: https://github.com/apache/nifi/pull/8123#issuecomment-1842625796 Hi @exceptionfactory ! Thanks for taking care! The separation of the default setting sounds a great idea, I will apply it with other requested changes. As for the API changes, I looked around and the usage of the *Status classes looks to be limited to some dedicated things. It did not seem harmful, however in the other hand in many times it travels together with the capture time of the status making it somewhat coincidental. An other idea I had (to keep the API intact) is to provide a wrapper class extending the original ones with this information but as I see it would come with a the creation of a huge amount of unnecessary object creation. I am open for ideas but this looked the most fitting solution so far -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
joewitt commented on PR #8123: URL: https://github.com/apache/nifi/pull/8123#issuecomment-1841517110 I have questions about the direction in general so I will take those over to the JIRA. This PR obviously was a ton of work and my questions/thoughts aren't about that right now so I think the JIRA is a better place to discuss/share those. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12236 Improving fault tolerancy of the QuestDB backed metrics repository [nifi]
exceptionfactory commented on code in PR #8123:
URL: https://github.com/apache/nifi/pull/8123#discussion_r1416185237
##
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/pom.xml:
##
@@ -108,7 +108,7 @@
10
-
org.apache.nifi.controller.status.history.VolatileComponentStatusRepository
+
org.apache.nifi.controller.status.history.questdb.EmbeddedQuestDbStatusHistoryRepository
Review Comment:
I recommend reverting this particular change for this pull request. There is
a good deal to review, so if we get to the place where this is ready to be the
default setting, we should consider that after reviewing the substantive
changes.
##
nifi-api/src/main/java/org/apache/nifi/controller/status/ConnectionStatus.java:
##
@@ -22,6 +22,7 @@
/**
*/
public class ConnectionStatus implements Cloneable {
+private long createdAtInMs;
Review Comment:
This addition raises several questions and concerns. It does not seem like
the NiFi API definition should include the creation timestamp of components in
all circumstances. It would make much more sense to track this as a feature
internal to the framework.
As far as naming the `InMs` suffix is awkward, and it would be much better
to use `java.time.Instant` for model classes.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
