Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QPID-8666: [Broker-J] Broker plugin jdbc-provider-bone replacement #238

Merged
merged 3 commits into from
Feb 2, 2024

Conversation

dakirily
Copy link
Contributor

@dakirily dakirily commented Feb 1, 2024

This PR is a follow-up to the PR 235 and addresses JIRA QPID-8666, adding upgrading the broker model version from 9.0 to 9.1

@vavrtom vavrtom merged commit f154ea9 into apache:main Feb 2, 2024
4 checks passed
Comment on lines +127 to +130
"qpid.jdbcstore.bonecp.maxConnectionsPerPartition", 7,
"qpid.jdbcstore.bonecp.minConnectionsPerPartition", 6,
"qpid.jdbcstore.bonecp.partitionCount", 2);
final Map<String,Object> expectedAttributes = Map.of("connectionPoolType", "BONECP",
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Robbie,

PR-239 was created to fix those issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants