[
https://issues.apache.org/jira/browse/KNOX-2835?focusedWorklogId=824592&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-824592
]
ASF GitHub Bot logged work on KNOX-2835:
----------------------------------------
Author: ASF GitHub Bot
Created on: 09/Nov/22 14:00
Start Date: 09/Nov/22 14:00
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #670:
URL: https://github.com/apache/knox/pull/670#discussion_r1017963443
##########
gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/DefaultConfigurationMonitorProvider.java:
##########
@@ -16,16 +16,60 @@
*/
package org.apache.knox.gateway.topology.monitor;
+import java.io.File;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Map;
+
import org.apache.knox.gateway.config.GatewayConfig;
-import
org.apache.knox.gateway.services.config.client.RemoteConfigurationRegistryClientService;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.factory.AbstractServiceFactory;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+import
org.apache.knox.gateway.topology.monitor.db.DbRemoteConfigurationMonitor;
+import org.apache.knox.gateway.topology.monitor.db.LocalDirectory;
+import org.apache.knox.gateway.topology.monitor.db.RemoteConfigDatabase;
+import org.apache.knox.gateway.util.JDBCUtils;
-public class DefaultConfigurationMonitorProvider implements
RemoteConfigurationMonitorProvider {
+public class DefaultConfigurationMonitorProvider extends
AbstractServiceFactory {
+ @Override
+ protected RemoteConfigurationMonitor createService(GatewayServices
gatewayServices,
+ ServiceType serviceType,
+ GatewayConfig gatewayConfig,
+ Map<String, String> options,
+ String implementation) throws
ServiceLifecycleException {
+ RemoteConfigurationMonitor service = null;
+ if (matchesImplementation(implementation,
ZkRemoteConfigurationMonitor.class)) {
+ service = new ZkRemoteConfigurationMonitor(gatewayConfig,
gatewayServices.getService(ServiceType.REMOTE_REGISTRY_CLIENT_SERVICE));
+ } else if (matchesImplementation(implementation,
DbRemoteConfigurationMonitor.class)) {
+ service = createDbBasedMonitor(gatewayConfig,
gatewayServices.getService(ServiceType.ALIAS_SERVICE));
Review Comment:
The abstract class has a protected method called `getAliasService` for the
same; please re-use that.
##########
gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/db/DbRemoteConfigurationMonitor.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.knox.gateway.topology.monitor.db;
+
+import static java.util.stream.Collectors.toSet;
+
+import java.io.IOException;
+import java.time.Instant;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.knox.gateway.GatewayMessages;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.topology.monitor.RemoteConfigurationMonitor;
+
+public class DbRemoteConfigurationMonitor implements
RemoteConfigurationMonitor {
+ private static final GatewayMessages LOG =
MessagesFactory.get(GatewayMessages.class);
+ public static final int OFFSET_SECONDS = 5;
+ private final RemoteConfigDatabase db;
+ private final LocalDirectory providersDir;
+ private final LocalDirectory descriptorsDir;
+ private long intervalSeconds;
+ private final ScheduledExecutorService executor;
+ private Instant lastSyncTime;
+
+ public DbRemoteConfigurationMonitor(RemoteConfigDatabase db, LocalDirectory
providersDir, LocalDirectory descriptorsDir, long intervalSeconds) {
+ this.db = db;
+ this.providersDir = providersDir;
+ this.descriptorsDir = descriptorsDir;
+ this.executor = Executors.newSingleThreadScheduledExecutor();
+ this.intervalSeconds = intervalSeconds;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws
ServiceLifecycleException {}
+
+ @Override
+ public void start() throws ServiceLifecycleException {
+ LOG.startingDbRemoteConfigurationMonitor(intervalSeconds);
+ executor.scheduleWithFixedDelay(this::sync, intervalSeconds,
intervalSeconds, TimeUnit.SECONDS);
+ }
+
+ @Override
+ public void stop() throws ServiceLifecycleException {
+ executor.shutdown();
+ }
+
+ @Override
+ public boolean createProvider(String name, String content) {
+ return db.putProvider(name, content);
+ }
+
+ @Override
+ public boolean createDescriptor(String name, String content) {
+ return db.putDescriptor(name, content);
+ }
+
+ @Override
+ public boolean deleteProvider(String name) {
+ return db.deleteProvider(name);
+ }
+
+ @Override
+ public boolean deleteDescriptor(String name) {
+ return db.deleteDescriptor(name);
+ }
+
+ public void sync() {
+ try {
+ syncLocalWithRemote(db.selectProviders(), providersDir);
+ syncLocalWithRemote(db.selectDescriptors(), descriptorsDir);
+ lastSyncTime = Instant.now();
Review Comment:
You may add a DEBUG-level log message with the fact that sync was done.
##########
gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/db/DbRemoteConfigurationMonitor.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.knox.gateway.topology.monitor.db;
+
+import static java.util.stream.Collectors.toSet;
+
+import java.io.IOException;
+import java.time.Instant;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.knox.gateway.GatewayMessages;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.topology.monitor.RemoteConfigurationMonitor;
+
+public class DbRemoteConfigurationMonitor implements
RemoteConfigurationMonitor {
+ private static final GatewayMessages LOG =
MessagesFactory.get(GatewayMessages.class);
+ public static final int OFFSET_SECONDS = 5;
+ private final RemoteConfigDatabase db;
+ private final LocalDirectory providersDir;
+ private final LocalDirectory descriptorsDir;
+ private long intervalSeconds;
+ private final ScheduledExecutorService executor;
+ private Instant lastSyncTime;
+
+ public DbRemoteConfigurationMonitor(RemoteConfigDatabase db, LocalDirectory
providersDir, LocalDirectory descriptorsDir, long intervalSeconds) {
+ this.db = db;
+ this.providersDir = providersDir;
+ this.descriptorsDir = descriptorsDir;
+ this.executor = Executors.newSingleThreadScheduledExecutor();
+ this.intervalSeconds = intervalSeconds;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws
ServiceLifecycleException {}
+
+ @Override
+ public void start() throws ServiceLifecycleException {
+ LOG.startingDbRemoteConfigurationMonitor(intervalSeconds);
+ executor.scheduleWithFixedDelay(this::sync, intervalSeconds,
intervalSeconds, TimeUnit.SECONDS);
+ }
+
+ @Override
+ public void stop() throws ServiceLifecycleException {
+ executor.shutdown();
+ }
+
+ @Override
+ public boolean createProvider(String name, String content) {
+ return db.putProvider(name, content);
+ }
+
+ @Override
+ public boolean createDescriptor(String name, String content) {
+ return db.putDescriptor(name, content);
+ }
+
+ @Override
+ public boolean deleteProvider(String name) {
+ return db.deleteProvider(name);
+ }
+
+ @Override
+ public boolean deleteDescriptor(String name) {
+ return db.deleteDescriptor(name);
Review Comment:
DEBUG-level log about removing a descriptor from the DB.
##########
gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/DefaultConfigurationMonitorProvider.java:
##########
@@ -16,16 +16,60 @@
*/
package org.apache.knox.gateway.topology.monitor;
+import java.io.File;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Map;
+
import org.apache.knox.gateway.config.GatewayConfig;
-import
org.apache.knox.gateway.services.config.client.RemoteConfigurationRegistryClientService;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.factory.AbstractServiceFactory;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+import
org.apache.knox.gateway.topology.monitor.db.DbRemoteConfigurationMonitor;
+import org.apache.knox.gateway.topology.monitor.db.LocalDirectory;
+import org.apache.knox.gateway.topology.monitor.db.RemoteConfigDatabase;
+import org.apache.knox.gateway.util.JDBCUtils;
-public class DefaultConfigurationMonitorProvider implements
RemoteConfigurationMonitorProvider {
+public class DefaultConfigurationMonitorProvider extends
AbstractServiceFactory {
Review Comment:
I think this should be called a service factory from now on:
`RemoteConfigurationMonitorServiceFactory`.
##########
gateway-spi/src/main/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitor.java:
##########
@@ -16,14 +16,11 @@
*/
package org.apache.knox.gateway.topology.monitor;
-import
org.apache.knox.gateway.services.config.client.RemoteConfigurationRegistryClient;
-
-public interface RemoteConfigurationMonitor {
-
- void start() throws Exception;
-
- void stop() throws Exception;
-
- RemoteConfigurationRegistryClient getClient();
+import org.apache.knox.gateway.services.Service;
+public interface RemoteConfigurationMonitor extends Service {
Review Comment:
This should be called a service to align with current naming conventions:
`RemoteConfigurationMonitorService`
##########
gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/db/DbRemoteConfigurationMonitor.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.knox.gateway.topology.monitor.db;
+
+import static java.util.stream.Collectors.toSet;
+
+import java.io.IOException;
+import java.time.Instant;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.knox.gateway.GatewayMessages;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.topology.monitor.RemoteConfigurationMonitor;
+
+public class DbRemoteConfigurationMonitor implements
RemoteConfigurationMonitor {
+ private static final GatewayMessages LOG =
MessagesFactory.get(GatewayMessages.class);
+ public static final int OFFSET_SECONDS = 5;
+ private final RemoteConfigDatabase db;
+ private final LocalDirectory providersDir;
+ private final LocalDirectory descriptorsDir;
+ private long intervalSeconds;
+ private final ScheduledExecutorService executor;
+ private Instant lastSyncTime;
+
+ public DbRemoteConfigurationMonitor(RemoteConfigDatabase db, LocalDirectory
providersDir, LocalDirectory descriptorsDir, long intervalSeconds) {
+ this.db = db;
+ this.providersDir = providersDir;
+ this.descriptorsDir = descriptorsDir;
+ this.executor = Executors.newSingleThreadScheduledExecutor();
+ this.intervalSeconds = intervalSeconds;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws
ServiceLifecycleException {}
+
+ @Override
+ public void start() throws ServiceLifecycleException {
+ LOG.startingDbRemoteConfigurationMonitor(intervalSeconds);
+ executor.scheduleWithFixedDelay(this::sync, intervalSeconds,
intervalSeconds, TimeUnit.SECONDS);
+ }
+
+ @Override
+ public void stop() throws ServiceLifecycleException {
+ executor.shutdown();
+ }
+
+ @Override
+ public boolean createProvider(String name, String content) {
+ return db.putProvider(name, content);
Review Comment:
DEBUG-level log about creating the provider in the DB.
##########
gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/db/DbRemoteConfigurationMonitor.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.knox.gateway.topology.monitor.db;
+
+import static java.util.stream.Collectors.toSet;
+
+import java.io.IOException;
+import java.time.Instant;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.knox.gateway.GatewayMessages;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.topology.monitor.RemoteConfigurationMonitor;
+
+public class DbRemoteConfigurationMonitor implements
RemoteConfigurationMonitor {
+ private static final GatewayMessages LOG =
MessagesFactory.get(GatewayMessages.class);
+ public static final int OFFSET_SECONDS = 5;
+ private final RemoteConfigDatabase db;
+ private final LocalDirectory providersDir;
+ private final LocalDirectory descriptorsDir;
+ private long intervalSeconds;
+ private final ScheduledExecutorService executor;
+ private Instant lastSyncTime;
+
+ public DbRemoteConfigurationMonitor(RemoteConfigDatabase db, LocalDirectory
providersDir, LocalDirectory descriptorsDir, long intervalSeconds) {
+ this.db = db;
+ this.providersDir = providersDir;
+ this.descriptorsDir = descriptorsDir;
+ this.executor = Executors.newSingleThreadScheduledExecutor();
+ this.intervalSeconds = intervalSeconds;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws
ServiceLifecycleException {}
+
+ @Override
+ public void start() throws ServiceLifecycleException {
+ LOG.startingDbRemoteConfigurationMonitor(intervalSeconds);
+ executor.scheduleWithFixedDelay(this::sync, intervalSeconds,
intervalSeconds, TimeUnit.SECONDS);
+ }
+
+ @Override
+ public void stop() throws ServiceLifecycleException {
+ executor.shutdown();
+ }
+
+ @Override
+ public boolean createProvider(String name, String content) {
+ return db.putProvider(name, content);
+ }
+
+ @Override
+ public boolean createDescriptor(String name, String content) {
+ return db.putDescriptor(name, content);
Review Comment:
DEBUG-level log about creating the descriptor in the DB.
##########
gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/db/DbRemoteConfigurationMonitor.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.knox.gateway.topology.monitor.db;
+
+import static java.util.stream.Collectors.toSet;
+
+import java.io.IOException;
+import java.time.Instant;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.knox.gateway.GatewayMessages;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.topology.monitor.RemoteConfigurationMonitor;
+
+public class DbRemoteConfigurationMonitor implements
RemoteConfigurationMonitor {
+ private static final GatewayMessages LOG =
MessagesFactory.get(GatewayMessages.class);
+ public static final int OFFSET_SECONDS = 5;
+ private final RemoteConfigDatabase db;
+ private final LocalDirectory providersDir;
+ private final LocalDirectory descriptorsDir;
+ private long intervalSeconds;
+ private final ScheduledExecutorService executor;
+ private Instant lastSyncTime;
+
+ public DbRemoteConfigurationMonitor(RemoteConfigDatabase db, LocalDirectory
providersDir, LocalDirectory descriptorsDir, long intervalSeconds) {
+ this.db = db;
+ this.providersDir = providersDir;
+ this.descriptorsDir = descriptorsDir;
+ this.executor = Executors.newSingleThreadScheduledExecutor();
+ this.intervalSeconds = intervalSeconds;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws
ServiceLifecycleException {}
+
+ @Override
+ public void start() throws ServiceLifecycleException {
+ LOG.startingDbRemoteConfigurationMonitor(intervalSeconds);
+ executor.scheduleWithFixedDelay(this::sync, intervalSeconds,
intervalSeconds, TimeUnit.SECONDS);
+ }
+
+ @Override
+ public void stop() throws ServiceLifecycleException {
+ executor.shutdown();
+ }
+
+ @Override
+ public boolean createProvider(String name, String content) {
+ return db.putProvider(name, content);
+ }
+
+ @Override
+ public boolean createDescriptor(String name, String content) {
+ return db.putDescriptor(name, content);
+ }
+
+ @Override
+ public boolean deleteProvider(String name) {
+ return db.deleteProvider(name);
Review Comment:
DEBUG-level log about removing the provider from the DB.
Issue Time Tracking
-------------------
Worklog Id: (was: 824592)
Time Spent: 20m (was: 10m)
> SQL DB based topology monitor
> -----------------------------
>
> Key: KNOX-2835
> URL: https://issues.apache.org/jira/browse/KNOX-2835
> Project: Apache Knox
> Issue Type: New Feature
> Reporter: Attila Magyar
> Assignee: Attila Magyar
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> This is similar to KNOX-1012 but the providers/descriptors are stored in the
> database instead of zookeeper.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
