----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7103/#review11595 -----------------------------------------------------------
Hey man, thanks for the patch. I believe this is related to the snappy issue raised over in HADOOP-8806, so it should be the java.library.path added to LD_LIBRARY_PATH, not the classpath. In the script, it's called FLUME_JAVA_LIBRARY_PATH. Aside from that, I have a couple comments: 1. The existing LD_LIBRARY_PATH should be discarded. There is a risk of an environment difference between system startup scripts and user shell invocations if we don't force environment variables that we depend on to be defined in the flume-env.sh script. So I think we should get rid of the logic involving SYSTEM_LD_LIBRARY_PATH. The problem occurs when an administrator sets LD_LIBRARY_PATH in his/her shell (maybe in their .bash_profile) and starts Flume, it runs fine for some time, and when the system reboots that variable is not set so Flume crashes on startup. 2. The body of the set_LD_LIBRARY_PATH() function can be indented, and I think it should be, for readability. - Mike Percy On Sept. 14, 2012, 7:41 a.m., Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7103/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2012, 7:41 a.m.) > > > Review request for Flume. > > > Description > ------- > > Added a method to add FLUME_CLASSPATH to LD_LIBRARY_PATH. > > > This addresses bug FLUME-1582. > https://issues.apache.org/jira/browse/FLUME-1582 > > > Diffs > ----- > > bin/flume-ng 121adf3 > > Diff: https://reviews.apache.org/r/7103/diff/ > > > Testing > ------- > > Ran an agent and checked the LD_LIBRARY_PATH. > > > Thanks, > > Hari Shreedharan > >
