gemmellr commented on code in PR #238:
URL: https://github.com/apache/qpid-broker-j/pull/238#discussion_r1476106471


##########
broker-core/src/test/java/org/apache/qpid/server/store/BrokerStoreUpgraderAndRecovererTest.java:
##########
@@ -121,9 +124,10 @@ public void 
testUpgradeVirtualHostWithJDBCStoreAndHikariCPPool()
                 "qpid.jdbcstore.varBinaryType", "myvarbinary",
                 "qpid.jdbcstore.blobType", "myblob",
                 "qpid.jdbcstore.useBytesForBlob", true,
-                "qpid.jdbcstore.hikaricp.maximumPoolSize", 7,
-                "qpid.jdbcstore.hikaricp.minimumIdle", 6);
-        final Map<String,Object> expectedAttributes = 
Map.of("connectionPoolType", "HIKARICP",
+                "qpid.jdbcstore.bonecp.maxConnectionsPerPartition", 7,
+                "qpid.jdbcstore.bonecp.minConnectionsPerPartition", 6,
+                "qpid.jdbcstore.bonecp.partitionCount", 2);
+        final Map<String,Object> expectedAttributes = 
Map.of("connectionPoolType", "BONECP",

Review Comment:
   Doesnt the fact these bits of the output didnt change (now restored to their 
previous/original values), even though the updater is meant to be handling 
transition from bonecp to hikaricp pool, suggest that not everything that 
should be upgraded is being so?
   
   EDIT: so this test is checking for "VirtualHostNode" context, whereas the 
updater is only covering "VirtualHost" context entries, which seems likely to 
explain things. Given both can apparently separately be using JDBC and thus 
pooling (even if I dont see a point for the node) it feels like both should be 
covered.
   
   EDIT2: Actually, can the broker have this context config as well? to be 
defined in one place and inherited by the rest if they use it and not override?



-- 
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: dev-unsubscr...@qpid.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

Reply via email to