INTEXT-36 - Voldemort Adapter #20

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

lukasz-antoniak commented Jan 13, 2013

Patch for INTEXT-36 JIRA ticket.
Link: https://jira.springsource.org/browse/INTEXT-36

Regards,
Lukasz Antoniak

Member

amolnayak311 commented Jan 15, 2013

  • Remove all jars from the project, these need to be a part of the dependencies and should not be placed in the repository.
  • Change the year in all the class to 2013 for the Licence in the header.
Contributor

lukasz-antoniak commented Jan 15, 2013

@amolnayak311: No, you cannot exclude those jar libraries. Voldemort is not build with Maven, and is not being uploaded to Maven Central Repository. Those libraries are required for integration tests to run against local Voldemort database. Otherwise, you would have to download them manually, and simple gradle test would not work after clean checkout.

Will change the year to 2013 shortly.

Member

amolnayak311 commented Jan 15, 2013

The commons, jackson, thift, jetty and I guess all others can come from maven repo. For voldemort jars, dont worry they can be put in Spring source's mvn repository. So eventually we will have to get the jars from the repository and none with the project's source code.

Contributor

lukasz-antoniak commented Jan 16, 2013

Got your point and it makes sense. What do you think about referencing Voldemort 0.96 with something like compile "voldemort:voldemort:$voldemortVersion", plus installing the following POM descriptor: https://gist.github.com/4546021 (all dependencies included)? I have tested it locally and everything works fine.

If you agree, could you please install Voldemort 0.96 with the given POM in Spring's Maven repository? I will test it afterwards and update build.gradle, remove libraries and .gitignore file.

Contributor

lukasz-antoniak commented Jan 29, 2013

Hello Spring Team, any recent updates?

Member

ghillert commented Jan 29, 2013

Hi Lukasz,

I have just added Voldemort to the SpringSource repository - voldemort:voldemort:0.96.

Cheers,

Gunnar

Contributor

lukasz-antoniak commented Jan 29, 2013

Thanks Gunnar, looks good from my working directory :).

Contributor

lukasz-antoniak commented Feb 13, 2013

Hello Spring Team,

In my opinion message store cannot be easily implemented, because Voldemort does not support transactions and row locking - AP system in terms of CAP theorem. During functional testing, I faced issues with concurrent modifications of single message group by two separate message store clients. BTW, any updates regarding code review and documentation?

Regards,
Lukasz

Member

artembilan commented Feb 13, 2013

Hello, Lukasz.
As you may know MongoDB doesn't support TX as well, however we have its MessageStore implementation.
From other side it is anti-pattern to access to the same MessageGroup from different applications...
To achieve exclusive access to the MessageGroup we use LockRegistry: take a look into AbstractCorrelatingMessageHandler.

Cheers

+ * @author Lukasz Antoniak
+ * @since 1.0
+ */
+public interface VoldemortConverter<K, V, P> {
@amolnayak311

amolnayak311 Feb 13, 2013

Member

Any reason for introducing this new interface and not using _o.s.i.s.c.MessageConverter_ instead?

@lukasz-antoniak

lukasz-antoniak Feb 14, 2013

Contributor

The main reason standing behind this decision was type safety and clarity. I didn't want user to realize in runtime (or while reading documentation) that the object he receives in MessageConverter#toMessage(Object) method is actually KeyValue<K, Versioned<V>> wrapper, and that I expect him to return KeyValue<K, V> in MessageConverter#fromMessage(Message<P>). If you can think of any better solution or using MessageConverter is preferred, I'll be happy to alter this part of code. I can see that spring-integration-mongodb module also uses MongoDB specific converter.

@amolnayak311

amolnayak311 Feb 14, 2013

Member

What you say makes sense and yes, Mongo adapters do use a custom converter, but that comes from spring data mongo and is used to instantiate the MongoTemplate if one is not provided.
I am comparing this more to the Redis's _o.s.i.r.oRedisStoreWritingMessageHandler_ as that would be a better comparison.
Merging in a way you have implemented shouldn't be a issue in my opinion as we can anytime change things in future to accommodate the changes.But do take a look at the way Redis's adapters are implemented and let us know your views.

@lukasz-antoniak

lukasz-antoniak Feb 22, 2013

Contributor

VoldemortConverter allows user to store chosen part of message payload. After quick skip through the implementation of RedisStoreWritingMessageHandler, I guess that it can persist only complete payload objects. Currently I think that payload transformation could be processed by @Transformer outside of Voldemort adapter, so let's omit this issue. More important design decision is the way of determining object's key. This is achieved programmatically in VoldemortConverter, while Redis adapter looks up concrete message header (RedisHeaders.KEY) or evaluates key-expression.

What would be the preferred way of passing/calculating object's key in your opinion? In case it's Redis' approach, I will change Voldemort adapter to work similarly. IMO +1 for Redis.

Member

amolnayak311 commented Feb 21, 2013

@lukasz-antoniak Apologies for not getting this merged quickly. I want to go through this and review it but I am tied up with some other stuff and unable to do this. Please do not feel that your contribution is not valued :)
It might take some time but it should be merged. Somebody else might review too as per their convenience.
Thanks again for the contribution.

Contributor

lukasz-antoniak commented Mar 1, 2013

I have refactored Voldemort adapter to calculate entry key analogically to Redis module. IMO design looks better now, as there is no extra interface and all you need to code is something like store-key-expression="payload.id". Thank you @amolnayak311 for this suggestion. Any progress with reviewing the rest of the code?

Contributor

lukasz-antoniak commented Mar 28, 2013

Hello Spring Team, any recent updates?

ghillert pushed a commit to ghillert/spring-integration-extensions that referenced this pull request Apr 2, 2013

Gunnar Hillert
Merge pull request #20 from lukasz-antoniak/INTEXT-36
* lukasz-antoniak-INTEXT-36:
  INTEXT-61 - Add Voldemort Sample
  INTEXT-36 - Cleanup
  INTEXT-36 - Add Voldemort Module
Member

ghillert commented Apr 2, 2013

Did some code cleanup:

  • Convert spaces to tabs
  • Upgrade to Gradle 1.5
  • Change Group Id to 'org.springframework.integration'
  • Add Sonar Runner plugin
  • Reduce Sonar warnings
  • Ensure ASL license header is present in all source files
  • Fix XSD documentation not showing up in STS

Also, I added a (very simple) Voldemort sample to the repo:

Furthermore, I have setup the CI infrastructure:

The Sonar Analytics dashboard is here:

The snapshot artifacts are automatically deployed to:

THANK YOU for the contribution! Looking forward to new PRs!

@ghillert ghillert closed this Apr 2, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment