[GitHub] storm issue #2518: STORM-2902: Some improvements for storm-rocketmq module

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/storm/pull/2518
  
Good Catch, what's the plan for this pr? I noticed that we have kept the 
unmerged status for a long time.


---

Storm multilang - .net core (WAS: [apache/storm] .NET Core 2.0 multi-lang adapter (#2613))

[Mauro Giusti]

Jungtaek, everyone, I am very sorry that we missed your reply –

If you received an undeliverable message from Microsoft, could you please 
forward to my private email address (mau...@live.com) ?
I need to make sure that this does not happen again –

I am now subscribed to the dev@storm.apache.org so 
I will not miss a reply to this 

Regarding the .net core multilang –
We understand the motivations and impact to the storm release, we agree that it 
is better hosted on a separate Git repo and will work towards that –
I like the idea of pointing to our .net adapter from the documentation and will 
work towards pushing that change to the docs to you folks -

About your initial feedback –
We understand the pain of making contribution that are not maintained.
It is a hard balance, between individuals that are trying to do the right thing 
and changing missions and projects over time – some personal skin in the game 
needs to go in there.
Having language-native ways to interact with storm would be a great benefit, 
since right now using the shell to communicate creates a big performance 
bottleneck with serialization/deserialization – if the project is making 
progress in this direction we will consider it for sure.

Thank you -
-MG-

From: Mauro Giusti
Sent: Tuesday, April 3, 2018 9:16 AM
To: apache/storm 
;
 apache/storm 
Cc: Mention 
Subject: RE: [apache/storm] .NET Core 2.0 multi-lang adapter (#2613)

Darn –
Did not realize that you replied but we did not receive the mail –
Looking at the thread now -

-MG-

From: Jungtaek Lim >
Sent: Monday, April 2, 2018 4:10 PM
To: apache/storm >
Cc: Mauro Giusti >; Mention 
>
Subject: Re: [apache/storm] .NET Core 2.0 multi-lang adapter (#2613)


@oizu
 
@MaurGi
Hello Azure team, please follow up the Storm developer mailing list since such 
discussion is taking place. If you're not subscribing dev mailing list, here's 
a link for tracking thread.

https://mail-archives.apache.org/mod_mbox/storm-dev/201804.mbox/%3c59aff170-49de-4f83-a7b9-23616da21...@gmail.com%3E

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on 
GitHub,
 or mute the 
thread.

[GitHub] storm issue #2591: STORM-2979: WorkerHooks EOFException during run_worker_sh...

[hummelm]

Github user hummelm commented on the issue:

https://github.com/apache/storm/pull/2591
  
Here is a new implementation which is compatible with stateful workerhook.
I think this implementation is better isn't it ?


---

[GitHub] storm pull request #2621: [STORM-3017] Refactor pacemaker client exception h...

[Ethanlm]

GitHub user Ethanlm opened a pull request:

https://github.com/apache/storm/pull/2621

[STORM-3017] Refactor pacemaker client exception handling

https://issues.apache.org/jira/browse/STORM-3017

Better exception handling for pacemaker client

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Ethanlm/storm STORM-3017

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/2621.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2621


commit 8b9173081045b31aff4064af3a44ce502c658c18
Author: Ethan Li 
Date:   2018-04-03T19:52:36Z

[STORM-3017] Refactor pacemaker client exception handling




---

[GitHub] storm issue #2620: STORM-2994: KafkaSpout doesn't commit offsets for null tu...

[reiabreu]

Github user reiabreu commented on the issue:

https://github.com/apache/storm/pull/2620
  
Yeah, going through the Travis logs it does seem they are unstable.
Thank you for your help. Much appreciated.


---

[GitHub] storm issue #2620: STORM-2994: KafkaSpout doesn't commit offsets for null tu...

[srdo]

Github user srdo commented on the issue:

https://github.com/apache/storm/pull/2620
  
Sorry, the tests sadly are very unstable on travis on the 1.x branch. Ran 
the storm-kafka-client tests locally and verified that they work on Java 7. 


---

[GitHub] storm pull request #2620: STORM-2994: KafkaSpout doesn't commit offsets for ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/2620


---

[GitHub] storm pull request #2620: STORM-2994: KafkaSpout doesn't commit offsets for ...

[reiabreu]

GitHub user reiabreu reopened a pull request:

https://github.com/apache/storm/pull/2620

STORM-2994: KafkaSpout doesn't commit offsets for null tuples



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/reiabreu/storm 1.x-branch

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/2620.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2620


commit d8a37d8f4e1e4a5b42255d01f9743eed2a99424e
Author: Rui Abreu 
Date:   2018-04-03T09:21:05Z

STORM-2994: KafkaSpout consumes messages but doesn't commit offsets




---

[GitHub] storm pull request #2620: STORM-2994: KafkaSpout doesn't commit offsets for ...

[reiabreu]

Github user reiabreu closed the pull request at:

https://github.com/apache/storm/pull/2620


---

[GitHub] storm pull request #2620: STORM-2994: KafkaSpout doesn't commit offsets for ...

[reiabreu]

GitHub user reiabreu opened a pull request:

https://github.com/apache/storm/pull/2620

STORM-2994: KafkaSpout doesn't commit offsets for null tuples



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/reiabreu/storm 1.x-branch

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/2620.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2620


commit d8a37d8f4e1e4a5b42255d01f9743eed2a99424e
Author: Rui Abreu 
Date:   2018-04-03T09:21:05Z

STORM-2994: KafkaSpout consumes messages but doesn't commit offsets




---

[GitHub] storm issue #600: STORM-903: fix java.lang.ClassNotFoundException with org.a...

[d2r]

Github user d2r commented on the issue:

https://github.com/apache/storm/pull/600
  
I have closed this PR due to inactivity. Jira STORM-903 remains open. 
Please re-open or create a new PR if we want to fix this.


---

[GitHub] storm pull request #600: STORM-903: fix java.lang.ClassNotFoundException wit...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/600


---

[GitHub] storm issue #2593: STORM-2994 KafkaSpout doesn't commit null tuples offsets

[reiabreu]

Github user reiabreu commented on the issue:

https://github.com/apache/storm/pull/2593
  
@srdo Sure, I'll have a look


---

[GitHub] storm issue #2593: STORM-2994 KafkaSpout doesn't commit null tuples offsets

[srdo]

Github user srdo commented on the issue:

https://github.com/apache/storm/pull/2593
  
@reiabreu Merged to master. Looks like there are some small modifications 
to do to get it on to 1.x or earlier branches. Could you put up a PR against 
1.x as well (should be pretty easy with `git cherry-pick`)?


---

[GitHub] storm pull request #2593: STORM-2994 KafkaSpout doesn't commit null tuples o...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/2593


---

[GitHub] storm issue #2613: .NET Core 2.0 multi-lang adapter

[MaurGi]

Github user MaurGi commented on the issue:

https://github.com/apache/storm/pull/2613
  
Darn –
Did not realize that you replied but we did not receive the mail –
Looking at the thread now -

-MG-

From: Jungtaek Lim 
Sent: Monday, April 2, 2018 4:10 PM
To: apache/storm 
Cc: Mauro Giusti ; Mention 

Subject: Re: [apache/storm] .NET Core 2.0 multi-lang adapter (#2613)



@oizu
 
@MaurGi
Hello Azure team, please follow up the Storm developer mailing list since 
such discussion is taking place. If you're not subscribing dev mailing list, 
here's a link for tracking thread.


https://mail-archives.apache.org/mod_mbox/storm-dev/201804.mbox/%3c59aff170-49de-4f83-a7b9-23616da21...@gmail.com%3E

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on 
GitHub,
 or mute the 
thread.



---

[GitHub] storm issue #2618: [STORM-2905] Fix KeyNotFoundException when kill a storm a...

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/2618
  
Just FYI I files STORM-3020 to address the race that I just found.


---

[GitHub] storm issue #2618: [STORM-2905] Fix KeyNotFoundException when kill a storm a...

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/2618
  
@danny0405 

{{updateBlobs}} does not need to be guarded by a lock.  This is what I was 
talking about with the code being complex.

{{requestDownloadBaseTopologyBlobs}} is protected by a lock simply because 
of this non-thread safe code.


https://github.com/apache/storm/blob/402a371ccdb39ccd7146fe9743e91ca36fee6d15/storm-server/src/main/java/org/apache/storm/localizer/AsyncLocalizer.java#L222-L226

Part of this were written prior to the more to java8 so {{computeIfAbsent}} 
was not available.  Now that it is we could replace it and I believe remove the 
lock, but I would want to spend some time to be sure it was not accidentally 
protecting something else in there too.

{{requestDownloadTopologyBlobs}} looks like it does not need to be 
synchronized at all.  It must have been a mistake on my part, but it does look 
like it might be providing some protection to a bug in


https://github.com/apache/storm/blob/402a371ccdb39ccd7146fe9743e91ca36fee6d15/storm-server/src/main/java/org/apache/storm/localizer/AsyncLocalizer.java#L191-L195

Which is executing outside of a lock, but looks to not be thread safe.

Declaring {{updateBlobs}} as synchronized does absolutely noting except 
make it have conflicts with {{requestDownloadTopologyBlobs}} and 
{{requestDownloadBaseTopologyBlobs}}.  And if we are able to remove the locks 
there, then it will not be an issue at all.  {{updateBlobs}} is scheduled using 
{{scheduleWithFixedDelay}}


https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ScheduledExecutorService.html#scheduleWithFixedDelay(java.lang.Runnable,%20long,%20long,%20java.util.concurrent.TimeUnit)

The javadocs clearly states that the next execution only starts a fixed 
delay after the previous one finished.  There will only ever be one copy it 
running at a time.  Additionally everything it does is already asynchronous so 
would be happening on a separate thread. Making it synchronized would just slow 
things down.  

The having a blob disappear at the wrong time is a race that will always be 
in the system and we cannot fix it with synchronization because it is happening 
on separate servers.  The only thing we can do is to deal with it when it 
happens.  The way the current code deals with it is to try again later.  This 
means that a worker that is trying to come up for the first time will not come 
up until the blob is fully downloaded, but if we are trying to update the blob 
and it has disappeared we will simply keep the older version around until we 
don't need it any more.  Yes we may log some exceptions while we do it, but 
that is the worst thing that will happen.


---

[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus

[kishorvpatil]

Github user kishorvpatil commented on the issue:

https://github.com/apache/storm/pull/2603
  
@HeartSaVioR , I think changes in #2433 do take care of caching assignments 
within `InMemoryAssignmentBackend`, so the changes proposed in #2603 are no 
longer needed. I am closing this PR


---

[GitHub] storm pull request #2603: [STORM-3003] Adding Assignment caching to Nimbus

[kishorvpatil]

Github user kishorvpatil closed the pull request at:

https://github.com/apache/storm/pull/2603


---

[GitHub] storm pull request #2618: [STORM-2905] Fix KeyNotFoundException when kill a ...

[revans2]

Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/2618#discussion_r178852812
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/daemon/supervisor/Slot.java ---
@@ -497,7 +497,6 @@ static DynamicState 
cleanupCurrentContainer(DynamicState dynamicState, StaticSta
 assert(dynamicState.container.areAllProcessesDead());
 
 dynamicState.container.cleanUp();
-
staticState.localizer.releaseSlotFor(dynamicState.currentAssignment, 
staticState.port);
--- End diff --

That is not true.


https://github.com/danny0405/storm/blob/a4e659b5073794396ea23e3dd7b79c00536fc3fe/storm-server/src/main/java/org/apache/storm/localizer/AsyncLocalizer.java#L505-L511

The code first removes the base blobs reference counts, but then it 
decrements a reference count for other blobs too.


---

[GitHub] storm issue #2389: [STORM-2693] Storm heartbeats promotion

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2389
  
@danny0405 Let's close this.


---

[GitHub] storm issue #2593: STORM-2994 KafkaSpout doesn't commit null tuples offsets

[reiabreu]

Github user reiabreu commented on the issue:

https://github.com/apache/storm/pull/2593
  
Thank you for guiding me through the changes. I've squashed all the commits 
and pushed a fresh one


---