hmsonline / storm-cassandra-cql

Storm Cassandra Bridge built on CQL
Apache License 2.0
43 stars 38 forks source link

attempt to correct #37 - is CassandraCqlState leaking statments? #38

Closed ghost closed 9 years ago

ghost commented 9 years ago

Hello,

We are seeing degradation on performance with time. So we are believing we have a leak somewhere. Should not we have some sort of reset of the this.statements field in the end of the commit method :

https://github.com/hmsonline/storm-cassandra-cql/blob/master/src/main/java/com/hmsonline/trident/cql/CassandraCqlState.java#L56

this.statements.clear();

Thanks in advance, Lucas

boneill42 commented 9 years ago

Agree. Completely. I thought this was previously fixed, but it appears as though we lost it. Thank you.

boneill42 commented 9 years ago

Are you seeing degradation in a running topology or in tests?

I believe a new state is created (via the StateFactory) for each batch.
(so statements would not get re-used) Regardless, I think it is a good idea to clear after each batch.

pabrahamsson commented 9 years ago

Thanks Lucas,

New release available: https://oss.sonatype.org/content/repositories/releases/com/hmsonline/storm-cassandra-cql/0.2.5/storm-cassandra-cql-0.2.5.jar

On Mon, Feb 2, 2015 at 2:36 PM, boneill42 notifications@github.com wrote:

Are you seeing degradation in a running topology or in tests?

I believe a new state is created (via the StateFactory) for each batch.

(so statements would not get re-used) Regardless, I think it is a good idea to clear after each batch.

— Reply to this email directly or view it on GitHub https://github.com/hmsonline/storm-cassandra-cql/pull/38#issuecomment-72522147 .

ghost commented 9 years ago

In tests on AWS. It is slower and slower as time pass.

ghost commented 9 years ago

Thanks a lot. New release does not seem to bear the leak. Further testing going on. It does not seem that a new state is created with each batch in our config. We will investigate that.