New Implementation of the Connection-Cache #747

chrisdutz · 2023-01-17T11:28:59Z

New-implementation of the Connection Cache and removal of both the old connection-cache as well as the connection-pool.

In addition some types in the API module were updated:

The PlcDriverManager was renamed to DefaultPlcDriverManager
A new Type PlcDriverManager
A new Type PlcConnectionManager
PlcDriverManager contains a getConnectionManager() method
PlcDriverManager contains a static method getDefault() which returns an instance of the DefaultDriverManager
DefaultPlcDriverManager implements both interfaces

…ding the datatypes in case of raw address queries.

…read device telemetry from the PLC using MDP

…ch just contains the getConnection variants in preparation of building a drop-in replacement plc-connection cache.

…read device telemetry from the PLC using MDP - Added some more data to the example (now we have the memory usage in %)

- Mostly untested draft implementation ... tests are coming.

- Added some more tests

- Implemented the timeout for leases - Added some more tests for timeouts

- Updated the rest of the project to use the new cache and deleted the old implementations.

- Created 2 new interfaces PlcConnectionManager and PlcDriverManager - Renamed the previous PlcDriverManager to DefaultPlcDriverManager

…ection-cache-and-scraper-ng # Conflicts: # plc4go/protocols/knxnetip/readwrite/model/KnxManufacturer.go # plc4j/drivers/knxnetip/src/main/generated/org/apache/plc4x/java/knxnetip/readwrite/KnxManufacturer.java

…ection-cache-and-scraper-ng # Conflicts: # plc4j/tools/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityInterceptorTest.java

sruehl · 2023-01-17T11:51:18Z

plc4j/api/src/main/java/org/apache/plc4x/java/DefaultPlcDriverManager.java


-    private static final Logger LOGGER = LoggerFactory.getLogger(PlcDriverManager.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(org.apache.plc4x.java.DefaultPlcDriverManager.class);


Suggested change

private static final Logger LOGGER = LoggerFactory.getLogger(org.apache.plc4x.java.DefaultPlcDriverManager.class);

private static final Logger LOGGER = LoggerFactory.getLogger(DefaultPlcDriverManager.class);

sruehl · 2023-01-17T11:52:41Z

plc4j/api/src/main/java/org/apache/plc4x/java/api/PlcConnectionManager.java

+     * @return PlcConnection object.
+     * @throws PlcConnectionException an exception if the connection attempt failed.
+     */
+    PlcConnection getConnection(String url, PlcAuthentication authentication) throws PlcConnectionException;


@chrisdutz we should some day think about using the option pattern from Golang here to (the ones with the varargs options). This way we could combine the getConnection into one method.

Well that is incompatible with the try-with-resources pattern ...

At least I think so

Don't think that should be a Problem but maybe I don't see it :D

sruehl · 2023-01-17T11:54:19Z

plc4j/api/src/main/java/org/apache/plc4x/java/api/PlcDriverManager.java


-    PlcConnection create() throws PlcConnectionException;


I wonder if we should leave the create method as deprecated default implementation:

default PlcConnection create() { return getConnectionManager().create(); }

What for? I mean ... we're obviously not using it.

because it is part of the API package an might break exiting deployments from customers.

Let's agree on not calling them "customers" as they are missing one important aspect of a typical customer relation.
I would say that "customers" have the chance to come forward and announce this would break things for them. I would consider it important to have backward compatibility once we are at 1.0.0. But I wouldn't object your proposal. Feel free to add it.

sruehl · 2023-01-17T11:55:14Z

plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/protocol/AdsProtocolLogic.java

-            }
-            //if (configuration.isLoadSymbolAndDataTypeTables()) {
+                context.fireConnected();
+//                future.completeExceptionally(new PlcConnectionException(


is the outcommmented code here an accident?

No ... it's a first step to make it possible to still connect to an ADS without loading the symbol table (if you just use raw addresses) ...

sruehl · 2023-01-17T11:55:57Z

plc4j/examples/hello-ads-telemetry/pom.xml

+      </plugin>
+    </plugins>
+  </build>
+
 </project>


Suggested change

</project>

</project>

Seriously? Not willing to do a discussion on the usage of new-lines.

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_206

sruehl · 2023-01-17T11:56:28Z

plc4j/examples/hello-ads-telemetry/src/main/resources/logback.xml

@@ -27,7 +27,7 @@
    </encoder>
  </appender>

-  <root level="error">
+  <root level="info">


Suggested change

<root level="info">

<root level="error">

I'm using the logging to output information on INFO level ... what good would the application be, if we set the log level to Error and just stripped all usable output? If yes, then you should have suggested a logger entry to explicitly set the log level of the application to info, but this change doesn't make any sense to me.

Couple of months back I turned them all up to error as we just exceeded the logoutput because of the size of our build. Those changes where all in the test-logback.xml. Just noticed that this is the main logback file so that should be fine so nvm

sruehl · 2023-01-17T11:58:04Z

...ection-cache/src/main/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManager.java

+
+    public PlcConnection getConnection(String url) throws PlcConnectionException {
+        ConnectionContainer connectionContainer;
+        synchronized (connectionContainers) {


an alternative here might be the use of the ConcurrentHashMap

sruehl · 2023-01-17T11:59:22Z

...on-cache/src/test/java/org/apache/plc4x/java/utils/cache/CachedPlcConnectionManagerTest.java

+     */
+    @Test
+    public void testSingleConnectionRequestTest() throws PlcConnectionException {
+        PlcConnectionManager mockConnectionManager = Mockito.mock(PlcConnectionManager.class);


There is also the Mockito Annotation which is pretty cool to use

sruehl · 2023-01-17T11:59:44Z

plc4j/tools/connection-cache/src/test/resources/logback-test.xml

-  <root level="error">
+  <logger name="org.apache.plc4x.java.utils.cache" level="debug"/>
+
+  <root level="info">


Suggested change

<root level="info">

<root level="error">

sruehl · 2023-01-17T11:59:53Z

plc4j/tools/connection-cache/src/test/resources/logback-test.xml

@@ -27,7 +27,9 @@
    </encoder>
  </appender>

-  <root level="error">
+  <logger name="org.apache.plc4x.java.utils.cache" level="debug"/>


Suggested change

<logger name="org.apache.plc4x.java.utils.cache" level="debug"/>

<logger name="org.apache.plc4x.java.utils.cache" level="error"/>

sruehl · 2023-01-17T12:00:34Z

plc4j/tools/scraper-ng/pom.xml

+    </dependency>
+  </dependencies>
+
+</project>


Suggested change

</project>

</project>

sruehl · 2023-01-17T12:00:52Z

plc4j/tools/scraper/pom.xml

-      <artifactId>plc4j-connection-cache</artifactId>
-      <version>0.11.0-SNAPSHOT</version>
-      <scope>compile</scope>
-    </dependency>
  </dependencies>

 </project>


Suggested change

</project>

</project>

sruehl · 2023-01-17T12:01:11Z

plc4j/tools/scraper-ng/pom.xml

+  <description>Utility to efficiently collect a large number of items on multiple devices by different triggers.</description>
+
+  <dependencies>
+    <!--Jackson-->


maybe remove those outcommented deps

sruehl

Left some suggestions which should be easy to apply

…ection-cache-and-scraper-ng

chrisdutz added 17 commits January 11, 2023 15:43

fix(plc4j/ads): Made it possible to use an ADS connection without loa…

52abe83

…ding the datatypes in case of raw address queries.

feat(plc4j/examples): Added a new example, using the ADS protocol to …

433a100

…read device telemetry from the PLC using MDP

feat(plc4j/api): Introduced a new PlcConnectionManager interface, whi…

b01d1fb

…ch just contains the getConnection variants in preparation of building a drop-in replacement plc-connection cache.

fix(build): Updated the plexus-compiler-ecplise plugin

fe24a4e

feat(plc4j/examples): Added a new example, using the ADS protocol to …

e50381b

…read device telemetry from the PLC using MDP - Added some more data to the example (now we have the memory usage in %)

feat(ams): Made it possible to decode error responses

3681ea9

feat(drivers): Updated vendor-ids and service ids

8d71fcb

feat(cache): Added a new implementation of a connection cache

56f2271

- Mostly untested draft implementation ... tests are coming.

feat(cache): Added a new implementation of a connection cache

2594ebe

- Added some more tests

feat(cache): Added a new implementation of a connection cache

c14140e

- Implemented the timeout for leases - Added some more tests for timeouts

feat(cache): Added a new implementation of a connection cache

d792114

- Updated the rest of the project to use the new cache and deleted the old implementations.

refactor(plc4j/api): Updated the PlcDriverManager API

edd1243

- Created 2 new interfaces PlcConnectionManager and PlcDriverManager - Renamed the previous PlcDriverManager to DefaultPlcDriverManager

feat(ads): Added additional Known Group Index for ADS over EtherCAT

00b989b

feat(plc4j/ads): Added a new example for reading data via AoE

08421d3

Merge remote-tracking branch 'origin/develop' into feature/cdutz/conn…

0d215a3

…ection-cache-and-scraper-ng # Conflicts: # plc4go/protocols/knxnetip/readwrite/model/KnxManufacturer.go # plc4j/drivers/knxnetip/src/main/generated/org/apache/plc4x/java/knxnetip/readwrite/KnxManufacturer.java

chore(build): Updated types after a full build

c2eeca1

Merge remote-tracking branch 'origin/develop' into feature/cdutz/conn…

eb201e6

…ection-cache-and-scraper-ng # Conflicts: # plc4j/tools/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityInterceptorTest.java

sruehl reviewed Jan 17, 2023

View reviewed changes

plc4j/tools/scraper-ng/pom.xml

</dependency>

</dependencies>

</project>

Copy link

Contributor

sruehl Jan 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

</project>

</project>

sruehl reviewed Jan 17, 2023

View reviewed changes

sruehl approved these changes Jan 17, 2023

View reviewed changes

chrisdutz added 4 commits January 17, 2023 13:10

docs(plc4j/ads): Added a comment on the next steps with the ADS driver

87ace44

Merge remote-tracking branch 'origin/develop' into feature/cdutz/conn…

b07e811

…ection-cache-and-scraper-ng

Merge remote-tracking branch 'origin/develop' into feature/cdutz/conn…

7be0d38

…ection-cache-and-scraper-ng

chore(infra): Redirected the issue notifications to dev for testing.

568d347

chrisdutz merged commit b3b8513 into develop Jan 21, 2023

chrisdutz deleted the feature/cdutz/connection-cache-and-scraper-ng branch January 21, 2023 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Implementation of the Connection-Cache #747

New Implementation of the Connection-Cache #747

chrisdutz commented Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

chrisdutz Jan 17, 2023

chrisdutz Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

chrisdutz Jan 17, 2023

sruehl Jan 17, 2023

chrisdutz Jan 17, 2023

sruehl Jan 17, 2023

chrisdutz Jan 17, 2023

sruehl Jan 17, 2023

chrisdutz Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

chrisdutz Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

sruehl Jan 17, 2023

sruehl left a comment


		private static final Logger LOGGER = LoggerFactory.getLogger(PlcDriverManager.class);
		private static final Logger LOGGER = LoggerFactory.getLogger(org.apache.plc4x.java.DefaultPlcDriverManager.class);

	<logger name="org.apache.plc4x.java.utils.cache" level="debug"/>
	<logger name="org.apache.plc4x.java.utils.cache" level="error"/>

New Implementation of the Connection-Cache #747

New Implementation of the Connection-Cache #747

Conversation

chrisdutz commented Jan 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sruehl left a comment

Choose a reason for hiding this comment