Skip to content

MINOR: Fix multiple org.apache.kafka.streams.KafkaStreams.StreamStateListener being instantiated#2801

Closed
original-brownbear wants to merge 2 commits intoapache:trunkfrom
original-brownbear:fix-stream-state-listener
Closed

MINOR: Fix multiple org.apache.kafka.streams.KafkaStreams.StreamStateListener being instantiated#2801
original-brownbear wants to merge 2 commits intoapache:trunkfrom
original-brownbear:fix-stream-state-listener

Conversation

@original-brownbear
Copy link
Copy Markdown
Member

There should only be a single org.apache.kafka.streams.KafkaStreams.StreamStateListener to ensure synchronization of operations on org.apache.kafka.streams.KafkaStreams.StreamStateListener#threadState.

@original-brownbear
Copy link
Copy Markdown
Member Author

@ijuma @dguy ping :) Hope I understood you guys correctly and this is what you had in mind here: #2791 (comment) :)

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/2677/
Test FAILed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/2677/
Test PASSed (JDK 8 and Scala 2.12).

Copy link
Copy Markdown
Contributor

@dguy dguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @original-brownbear - yes that is what i had in mind. Just one nit otherwise LGTM

globalThreadId);
}

StreamStateListener streamStateListener = new StreamStateListener(threadState);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: final

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/2681/
Test PASSed (JDK 8 and Scala 2.11).

@original-brownbear
Copy link
Copy Markdown
Member Author

@dguy alright :) final added :)

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/2682/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/2678/
Test PASSed (JDK 8 and Scala 2.12).

@original-brownbear
Copy link
Copy Markdown
Member Author

@dguy @ijuma hmm I wonder if this is related to this change, seems Scala 2.10 dead locked again in the streams ITs ... maybe there is some unexpected side-effect here? Looking into that.

@original-brownbear
Copy link
Copy Markdown
Member Author

@dguy @ijuma hmm never mind the above, I can't reproduce locking in RegexSourceIntegrationTest locally and the KafkaStreamsTest is still fine (on the same machine that was easily able to reproduce the other deadlock).
=> different issue I guess

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/2678/
Test FAILed (JDK 7 and Scala 2.10).

@asfgit asfgit closed this in 97e61d4 Apr 5, 2017
@original-brownbear original-brownbear deleted the fix-stream-state-listener branch April 5, 2017 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants