[
https://issues.apache.org/jira/browse/ARTEMIS-1447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16198366#comment-16198366
]
ASF GitHub Bot commented on ARTEMIS-1447:
-----------------------------------------
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1576
@clebertsuconic
> Instead of using the Journal Table for the locks... please create another
Table for that.
The node manager is already using a table different from the Journal Table
to store the locks/NodeId/state:
``// Default node manager store table name, used with Database storage type
private static final String DEFAULT_NODE_MANAGER_STORE_TABLE_NAME =
"NODE_MANAGER_STORE";``
> To be honest... I don't understand what's going on with the data.. and
this will be pretty hard to be maintained by someone else if you just use a
single table for things like this.
You're right, but I've implemented exactly the same data processing (and
layout) of
[FileLockNodeManager](https://github.com/apache/activemq-artemis/blob/master/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java),
hence I probably need to document it in both, given that is 1:1 with it: makes
sense?
>The package server.impl is already bloated.. I tried once to refactor it
into smaller packages...
This is a nice abstract model you have, but instead of the big class, cal
you create a packet jdbcNodeManager, and add your classes there?
Yes, good point: I'll do it :+1:
>Also, instead of creating your own scheduledExecutor.. Please extend
ActiveMQScheduledComponent... if you can reuse the same executors that we
already have created, just pass in the scheduledService as a parameter... and
if you must create a new ScheduledService, you can still use the same class for
that.
I'm not sure about it: it's similar to the TimedBuffer flusher thread.
It (they) needs to not being slowed down by anything in order to work as
expected and I've put several checks that will be triggered otherwise.
If we have already something similar that I can reuse I will do it for sure.
> Wouldn't be better to have a single statement (Using a separate table as
I had already asked).. using prepared statements?
Each line can be used for just one purpose and you can't have more than one
line doing it, exactly as the file based NodeManager: IMHO is more
dangerous/complex to have a prepared statement here.
>I'm a bit concerned also that the semantic of 1, 2 and 3.. is inside the
literal string.. and I see no references on the LeaseLock implementation. I
wouldn't understand how to debug this.. it makes it harder to maintain IMO.
Agree :+1: It is not clear and need more docs as I've answered above on
another point.
Considering how it works the only improvement I would do on the data layout
is to use 2 different tables,`NODE_MANAGER_STORE_LOCKS` and
`NODE_MANAGER_STORE_STATE`, to separate the locks and the shared state data.
TBH, considering that the first table will have just 2 rows and the second
just 1, probably it is not a big improvement that justify the change: keep it
simpler probably is a best deal.
> JDBC NodeManager to support JDBC HA Shared Store
> ------------------------------------------------
>
> Key: ARTEMIS-1447
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1447
> Project: ActiveMQ Artemis
> Issue Type: New Feature
> Components: Broker
> Reporter: Francesco Nigro
>
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
