qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Rudyy (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (QPID-5800) Refactoring: Introduce MessageStoreProvider interface to improve message/configuration store implementations
Date Mon, 09 Jun 2014 13:25:01 GMT

    [ https://issues.apache.org/jira/browse/QPID-5800?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14025175#comment-14025175
] 

Alex Rudyy edited comment on QPID-5800 at 6/9/14 1:23 PM:
----------------------------------------------------------

Keith,
I reviewed the changes made as part of this JIRA.
Here are my review comments:

DurableConfigurationStore:
Methods openConfigurationStore and closeConfigurationStore can be renamed into open/close
accordingly.
The same for MessageStore open and close methods. 

BDBConfigurationStore:
The message store field in BDBConfigurationStore is named "_messageStoreFacade". I would rename
that into "_messageStore"

BDBMessageStore is not fully split into BDBConfigurationStore and BDBMessageStore.
IMHO, such methods like getMessageContentDb, getMessageMetaDataDb, getMessageMetaDataSeqDb
etc should be moved into BDBMessageStore
Fields like _committer, _messageStoreOpen belongs to the BDBMessageStore.
I am curious how easy it would be to move BDBMessageStore into a separate class?

 
DerbyMessageStore/JDBCMessageStore:
For consistency they can be renamed into ConfigurationStores like DerbyConfigurationStore
and JDBCConfigurationStore
MessageStore field is called _messageStoreFacade
MessageStoreWrapper can be moved into AbstractJDBCMessageStore and could be renamed into JDBCMessageStore
or something more appropriate in order to move the message store functionality into a separate
class later on.


was (Author: alex.rufous):
Keith,
I reviewed the changes made as part of this JIRA.
Here are my review comments:

DurableConfigurationStore:
Methods openConfigurationStore and closeConfigurationStore can be renamed into open/close
accordingly.
The same for MessageStore open and close methods. 

BDBConfigurationStore:
The message store field in BDBConfigurationStore is named "_messageStoreFacade". I would rename
that into "_messageStore"

BDBMessageStore is not fully split into BDBConfigurationStore and BDBMessageStore.
IMHO, such methods like getMessageContentDb, getMessageMetaDataDb, getMessageMetaDataSeqDb
etc should be moved into BDBMessageStore
Fields like _committer, _messageStoreOpen belongs to the BDBMessageStore.
I am curious how easy it would be to move BDBMessageStore into a separate class?

Is there any reason why openSequence code is not called in the try/catch block in order to
handle the environment restart exceptions like InsufficientReplicasException? I am guessing
that this operation could be replicated?

I see that openSequence is duplicated in both SEF and REF. It looks like we can move the identical
code into the abstract parent class.

Also, I am not sure why sequence is called MESSAGE_METADATA_SEQ_KEY instead of MESSAGE_ID_SEQUENCE
as the primary purpose of the sequence is to generate message id for the message instance?
 
DerbyMessageStore/JDBCMessageStore:
For consistency they can be renamed into ConfigurationStores like DerbyConfigurationStore
and JDBCConfigurationStore
MessageStore field is called _messageStoreFacade
MessageStoreWrapper can be moved into AbstractJDBCMessageStore and could be renamed into JDBCMessageStore
or something more appropriate in order to move the message store functionality into a separate
class later on.

> Refactoring: Introduce MessageStoreProvider interface to improve message/configuration
store implementations
> ------------------------------------------------------------------------------------------------------------
>
>                 Key: QPID-5800
>                 URL: https://issues.apache.org/jira/browse/QPID-5800
>             Project: Qpid
>          Issue Type: Improvement
>          Components: Java Broker
>            Reporter: Keith Wall
>            Assignee: Alex Rudyy
>             Fix For: 0.29
>
>
> A configuration store implementation that wishes to additionally provide a message store
must currently implements the {{MessageStore}} interface. This design has led to rather monolithic
implementations.
> This refactoring will introduce a {{MesageStoreProvider}} interface.  Configuration stores
implementations that wish to also expose a MessageStore will implement this interface.  They
can then provide the MessageStore store implementation as they see fit, hopefully leading
to better, more cohesive, code.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

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


Mime
View raw message