I just pushed my changes to Marton's "storm" branch. It is still open how to process with the following (please give feedback):
StormSpoutWrapper:
- do we still need "isRunning" and "cancel()"? The new API should make
them obsolete from my point of view.
- I would avoid "busy wait" in "next()" and apply a "not-emit" penalty
within the while-loop:
> long sleep = 1;
> while(!stormCollector.hasNext()) {
> Thread.sleep(sleep);
> sleep *= 2;
> spout.nextTuple();
> }
StormFiniteSpoutWrapper:
- remove member variable "isDefined" --> this is redundant information
and might cause bugs...
- can we remove the "tupleEmitted" flag? Maybe we can implement it
without it (nor sure though)
I am also working on a new implementation of StromSpoutWrapper and
StormSpoutCollector. I will push it into my own repository if finished
and tell you. It could replace the current implementation without the
"nasty" buffering Queue (which I don't like). However, we need to
discuss this alternative implementation first.
-Matthias
On 06/03/2015 03:32 PM, Szabó Péter wrote:
> ---------- Forwarded message ----------
> From: Szabó Péter <nemderogator...@gmail.com>
> Date: 2015-06-03 15:31 GMT+02:00
> Subject: Re: Discussion: Storm Comparability Layer
> To: Márton Balassi <balassi.mar...@gmail.com>
>
>
> Hey, Matthias,
>
> Of course, you can remove my last commit. I just wanted to remove the
> failing tests, and some unnecessary comments. Please do the latter it in
> your commit as well.
>
> As for StormSpoutCollector, I used Queue with LinkedList implementation,
> because the list we keep is a queue in nature: we put records into it, and
> remove the head from time to time. The collector implements iterator,
> because I wanted to use something like next() and hasNext() in the
> StormSpoutWrapper. I think emphasizing this iterator-nature makes the code
> more readable.
>
> Peter
>
> 2015-06-03 14:16 GMT+02:00 Márton Balassi <balassi.mar...@gmail.com>:
>
>> Hey Matthias,
>>
>> We can undo Peter's commit if that helps you and have yours instead. You
>> can simply remove that commit in a rebase. Besides this let us push to the
>> same branch with trying not to break the history, I will squash the commits
>> once again if it gets too bulky.
>>
>> I would like to bring the discussion to the mailing list, so the cummunity
>> is seeing that you are actively working on this. Are you OK with reposting
>> this thread to the dev mailing list?
>>
>> On Wed, Jun 3, 2015 at 2:09 PM, Matthias J. Sax <
>> mj...@informatik.hu-berlin.de> wrote:
>>
>>> Hi,
>>>
>>> I just saw, that Peter pushed a new commit. It makes it hard for me to
>>> push my changes. Can we undo the last commit?
>>>
>>> If I get it right, it removes StormFiniteSpoutWrapper and disables
>>> failing test only. Do we want to delete StormFiniteSpoutWrapper? I would
>>> rather keep it.
>>>
>>> -Matthias
>>>
>>> On 06/03/2015 01:58 PM, Matthias J. Sax wrote:
>>>> Hi,
>>>>
>>>> I have a few questions about the current status ("storm" branch from
>>>> Marton).
>>>>
>>>> StormSpoutCollector:
>>>> - is there any specify advantage in using a Queue instead of
>>>> LinkedList for the internal buffer?
>>>> - Why are us implementing Iterator interface and mark
>>>> flinkCollectionDelegates as private?
>>>> -> I would rather drop the interface and make the variable "package
>>>> private" to access it directly (avoids "unnecessary" method calls)
>>>>
>>>> StormSpoutWrapper:
>>>> - do we still need "isRunning" and "cancel()"? The new API should make
>>>> them obsolete from my point of view.
>>>> - I would avoid "busy wait" in "next()" and apply a "not-emit" penalty
>>>> within the while-loop:
>>>>
>>>>> long sleep = 1;
>>>>> while(!stormCollector.hasNext()) {
>>>>> Thread.sleep(sleep);
>>>>> sleep *= 2;
>>>>> spout.nextTuple();
>>>>> }
>>>>
>>>> StormFiniteSpoutWrapper:
>>>> - remove member variable "isDefined" --> this is redundant information
>>>> and might cause bugs...
>>>> - can we remove the "tupleEmitted" flag? Maybe we can implement it
>>>> without it (nor sure though)
>>>>
>>>>
>>>> I am also working on a new implementation of StormSpoutOutputWrapper. I
>>>> will push it into my own repository if finished and tell you. It could
>>>> replace the current implementation without the "nasty" buffering Queue
>>>> (which I don't like). However, we need to discuss this alternative
>>>> implementation first.
>>>>
>>>> Things I would like to push:
>>>>
>>>> I fixed the following tests (was already fixed in my branch but not
>>>> merged by Marton):
>>>> - StormBoltWrapperTest
>>>> - StormSpoutWrapperTest
>>>> - StormFiniteSpoutWrapperTest
>>>> - Added new Test class InfiniteTestSpout
>>>>
>>>> I also step throw the hole code, removed "unused" tag (which are not
>>>> necessary for public methods), corrected a few spelling mistakes is
>>>> comments, and did some other minor "improvements".
>>>>
>>>> Additionally, I "merged" my changes (after my rebase) that are different
>>>> to Peters changes. Peter and I discussed some of the rebase differences
>>>> and I "merged" my and his changes (we both agreed how to resolve the
>>>> differenced already).
>>>>
>>>> If it is ok, I will push it directly into Marton's git repository.
>>>>
>>>>
>>>>
>>>> -Matthias
>>>>
>>>
>>>
>>
>
signature.asc
Description: OpenPGP digital signature
