samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Navina Ramesh" <nram...@linkedin.com>
Subject Re: Review Request 36545: SAMZA-682 Refactor Coordinator stream messages
Date Thu, 30 Jul 2015 17:06:15 GMT


> On July 24, 2015, 6:01 p.m., Navina Ramesh wrote:
> > Thanks for picking this up! It feels good to look at a refactored code. 
> > 
> > One suggestion: Please run all the intergration test (including the zopkio tests)
before checking in this patch. I don't think we cleanly start and stop coordinator stream
producers/consumers in all the managers. Please verify that nothing is broken due to this
change.
> 
> József Márton Jung wrote:
>     I have difficulties running the zopkio tests. The error message is the following:

>     2015-07-27 11:36:44,608 zopkio.remote_host_helper [ERROR] Error: JAVA_HOME is not
set and could not be found.
>     
>     JAVA_HOME is set on my machine (in /etc/profile, so it is available system-wide),
echoing it outputs the path to Java installation. I don't knw what is wrong.
> 
> Navina Ramesh wrote:
>     I think the problem is with the remote host (host to which zopkio is trying to ssh)
not having JAVA_HOME set correctly. Are you running the test on a remote machine?
> 
> József Márton Jung wrote:
>     I'm trying to run the tests locally, so zopkio is ssh-ing to localhost. When I connect
to localhost through ssh and when I try "echo $JAVA_HOME", it prints the correct path to my
Java home. I'm clueless at the moment.
> 
> József Márton Jung wrote:
>     Okay, I figured out. It works. Yay! When deployment is going through SSH, it is a
non-interctive shell, therefore /etc/profile is not executed.
>     More details can be found here: http://askubuntu.com/questions/247738/why-is-etc-profile-not-invoked-for-non-login-shells
>     When I added the following line before the line '# If not running interactively,
don't do anything' in ~/.bashrc:
>     ```
>        export JAVA_HOME=/path/to/java/home
>     ```
>     Integration tests started working.
>     
>     Maybe it would be a good idea to mention this on the page where integration tests
are described.

Duh.. that's true! It didn't occur to me. Glad you were able to figure it out. 
Yeah. We should probably mention this in the documentation.


- Navina


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36545/#review92938
-----------------------------------------------------------


On July 27, 2015, 10:15 a.m., József Márton Jung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36545/
> -----------------------------------------------------------
> 
> (Updated July 27, 2015, 10:15 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> The following has been refactored: 
> 1. Static inner classes from CoordinatorStreamMessage has been extracted
> 2. Common functionality from CheckpointManager, ChangelogMappingManager and LocalityManager
has benn moved to a base class
> 
> 
> Diffs
> -----
> 
>   checkstyle/import-control.xml 6654319 
>   samza-core/src/main/java/org/apache/samza/checkpoint/CheckpointManager.java 7445996

>   samza-core/src/main/java/org/apache/samza/container/LocalityManager.java 55c258f 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/AbstractCoordinatorStreamManager.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java
e5ab4fb 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java
b1078bd 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java
92f8907 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamWriter.java
f769756 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/CoordinatorStreamMessage.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/Delete.java PRE-CREATION

>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetChangelogMapping.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetCheckpoint.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetConfig.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetContainerHostMapping.java
PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/job/model/JobModel.java ad6387d 
>   samza-core/src/main/java/org/apache/samza/storage/ChangelogPartitionManager.java 7d3409c

>   samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala f621611

>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 1c178a6 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java
e454593 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamMessage.java
ac26a01 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamSystemConsumer.java
c25f6a7 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamSystemProducer.java
1ef07d0 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamWriter.java
c484660 
>   samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 84fdeaa

>   samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 41303f7 
> 
> Diff: https://reviews.apache.org/r/36545/diff/
> 
> 
> Testing
> -------
> 
> Tests has been updated.
> 
> 
> Thanks,
> 
> József Márton Jung
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message