Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Please apply this patch, this is why log4j exists. Rerunning at non-deterministic command twice to catch error message is extremely dangerous.

      diff --git a/bin/kafka-run-class.sh b/bin/kafka-run-class.sh
      index eb6ff1b..2f2d8b5 100755
      — a/bin/kafka-run-class.sh
      +++ b/bin/kafka-run-class.sh
      @@ -102,19 +102,3 @@ if [ "$1" = "daemon" ] && [ -z "$KAFKA_GC_LOG_OPTS"] ; then
      fi

      $JAVA $KAFKA_HEAP_OPTS $KAFKA_JVM_PERFORMANCE_OPTS $KAFKA_GC_LOG_OPTS $KAFKA_JMX_OPTS $KAFKA_LOG4J_OPTS -cp $CLASSPATH $KAFKA_OPTS "$@"
      -
      -exitval=$?
      -
      -if [ $exitval -eq "1" ] ; then

      • $JAVA $KAFKA_HEAP_OPTS $KAFKA_JVM_PERFORMANCE_OPTS $KAFKA_GC_LOG_OPTS $KAFKA_JMX_OPTS $KAFKA_LOG4J_OPTS -cp $CLASSPATH $KAFKA_OPTS "$@" >& exception.txt
      • exception=`cat exception.txt`
      • noBuildMessage='Please build the project using sbt. Documentation is available at http://kafka.apache.org/'
      • pattern="(Could not find or load main class)|(java\.lang\.NoClassDefFoundError)"
      • match=`echo $exception | grep -E "$pattern"`
      • if [[ -n "$match" ]]; then
      • echo $noBuildMessage
      • fi
      • rm exception.txt
        -fi
        -
        -

        Activity

        Hide
        Jay Kreps added a comment -

        Checked in on both 0.8 and trunk.

        Show
        Jay Kreps added a comment - Checked in on both 0.8 and trunk.
        Hide
        Jun Rao added a comment -

        Thanks for the patch. +1. Could you patch for trunk as well?

        Show
        Jun Rao added a comment - Thanks for the patch. +1. Could you patch for trunk as well?
        Hide
        Joe Stein added a comment -

        +1 on KAFKA-1081-v1.patch

        Show
        Joe Stein added a comment - +1 on KAFKA-1081 -v1.patch
        Hide
        Jay Kreps added a comment -

        Here is a proposed patch that uses exec for all the main commands and removes the re-execution for errors.

        Show
        Jay Kreps added a comment - Here is a proposed patch that uses exec for all the main commands and removes the re-execution for errors.
        Hide
        Anatoly Fayngelerin added a comment -

        There is another reason why removing this behavior would be beneficial.

        Currently, the run-class script spawns a subprocess. This makes running it under supervisor more difficult than it needs to be. This likely applies to other process monitoring systems. I maintain an internal patch on top of kafka that prepends the java invocation in run-class with exec and removes the error reporting. In production, the error is not very useful.

        Show
        Anatoly Fayngelerin added a comment - There is another reason why removing this behavior would be beneficial. Currently, the run-class script spawns a subprocess. This makes running it under supervisor more difficult than it needs to be. This likely applies to other process monitoring systems. I maintain an internal patch on top of kafka that prepends the java invocation in run-class with exec and removes the error reporting. In production, the error is not very useful.
        Hide
        Jay Kreps added a comment -

        Yeah I agree, rerunning the command to get the error is really weird. Given that we have a binary release do we need to retain this behavior? I.e. if you download the source release and try to run without building then it won't work but this shouldn't be as confusing as it was when we only had a source release and didn't really label the download as such.

        Show
        Jay Kreps added a comment - Yeah I agree, rerunning the command to get the error is really weird. Given that we have a binary release do we need to retain this behavior? I.e. if you download the source release and try to run without building then it won't work but this shouldn't be as confusing as it was when we only had a source release and didn't really label the download as such.
        Hide
        Francois Saint-Jacques added a comment -

        I know you guys have a binary release, this is what I'm deploying. My point is, this specific snippet should never go in production release.

        Show
        Francois Saint-Jacques added a comment - I know you guys have a binary release, this is what I'm deploying. My point is, this specific snippet should never go in production release.
        Hide
        Joe Stein added a comment -

        We have binary release http://kafka.apache.org/downloads.html since 0.8.0-beta1 and moving forward, you could use that since it is already packaged. I would go so far as to even bootstrap the sbt launcher but that is a lot more changes than just a consistent error when not compiling source before run.

        Show
        Joe Stein added a comment - We have binary release http://kafka.apache.org/downloads.html since 0.8.0-beta1 and moving forward, you could use that since it is already packaged. I would go so far as to even bootstrap the sbt launcher but that is a lot more changes than just a consistent error when not compiling source before run.
        Hide
        Francois Saint-Jacques added a comment -

        Look, this is an ugly hack. The real problem here is not the directory permission or using a temp file but RERUNNING the first java command.

        I'm not even trying to build it, I'm trying to correctly package kafka on a production server. Whenever I run any command (bin/*) that doesn't return properly, it borks if kafka-run-class.sh is called within a directory where the user don't have write permission.

        I understand that these lines are there to help new users who checkout the project and forget to build before running any command, but we're talking of deploying quality code in production.

        Show
        Francois Saint-Jacques added a comment - Look, this is an ugly hack. The real problem here is not the directory permission or using a temp file but RERUNNING the first java command. I'm not even trying to build it, I'm trying to correctly package kafka on a production server. Whenever I run any command (bin/*) that doesn't return properly, it borks if kafka-run-class.sh is called within a directory where the user don't have write permission. I understand that these lines are there to help new users who checkout the project and forget to build before running any command, but we're talking of deploying quality code in production.
        Hide
        Joe Stein added a comment -

        I don't see removing that function as a solution, we could re work it so that it communicates the error that you need to build the project.

        i don't have permission issue writing exception.txt but could see another way of fixing the function to not use a file and keep it all in vars if you wanted to rework your patch or chmod a+rw in your folder maybe not sure how you are running things but you should build first (see README)

        Joes-MacBook-Air:kafka joestein$ bin/kafka-list-topic.sh
        Missing required argument "[zookeeper]"
        Option Description
        ------ -----------
        --topic <topic> REQUIRED: The topic to be listed.
        Defaults to all existing topics.
        (default: )
        --unavailable-partitions if set, only show partitions whose
        leader is not available
        --under-replicated-partitions if set, only show under replicated
        partitions
        --zookeeper <urls> REQUIRED: The connection string for
        the zookeeper connection in the form
        host:port. Multiple URLS can be
        given to allow fail-over.
        Joes-MacBook-Air:kafka joestein$

        Show
        Joe Stein added a comment - I don't see removing that function as a solution, we could re work it so that it communicates the error that you need to build the project. i don't have permission issue writing exception.txt but could see another way of fixing the function to not use a file and keep it all in vars if you wanted to rework your patch or chmod a+rw in your folder maybe not sure how you are running things but you should build first (see README) Joes-MacBook-Air:kafka joestein$ bin/kafka-list-topic.sh Missing required argument " [zookeeper] " Option Description ------ ----------- --topic <topic> REQUIRED: The topic to be listed. Defaults to all existing topics. (default: ) --unavailable-partitions if set, only show partitions whose leader is not available --under-replicated-partitions if set, only show under replicated partitions --zookeeper <urls> REQUIRED: The connection string for the zookeeper connection in the form host:port. Multiple URLS can be given to allow fail-over. Joes-MacBook-Air:kafka joestein$
        Hide
        Francois Saint-Jacques added a comment -

        Look for the 3 last lines, not the usage error.

        $ cd /
        $ /opt/kafka/bin/kafka-list-topic.sh
        Missing required argument "[zookeeper]"
        Option Description
        ------ -----------
        --topic <topic> REQUIRED: The topic to be listed.
        Defaults to all existing topics.
        (default: )
        --unavailable-partitions if set, only show partitions whose
        leader is not available
        --under-replicated-partitions if set, only show under replicated
        partitions
        --zookeeper <urls> REQUIRED: The connection string for
        the zookeeper connection in the form
        host:port. Multiple URLS can be
        given to allow fail-over.
        /opt/kafka/bin/kafka-run-class.sh: line 72: exception.txt: Permission denied
        cat: exception.txt: No such file or directory
        rm: cannot remove 'exception.txt': No such file or directory

        Show
        Francois Saint-Jacques added a comment - Look for the 3 last lines, not the usage error. $ cd / $ /opt/kafka/bin/kafka-list-topic.sh Missing required argument " [zookeeper] " Option Description ------ ----------- --topic <topic> REQUIRED: The topic to be listed. Defaults to all existing topics. (default: ) --unavailable-partitions if set, only show partitions whose leader is not available --under-replicated-partitions if set, only show under replicated partitions --zookeeper <urls> REQUIRED: The connection string for the zookeeper connection in the form host:port. Multiple URLS can be given to allow fail-over. /opt/kafka/bin/kafka-run-class.sh: line 72: exception.txt: Permission denied cat: exception.txt: No such file or directory rm: cannot remove 'exception.txt': No such file or directory
        Hide
        Joe Stein added a comment -

        please can you put together reproducible steps for the issue, thanks!

        Show
        Joe Stein added a comment - please can you put together reproducible steps for the issue, thanks!

          People

          • Assignee:
            Unassigned
            Reporter:
            Francois Saint-Jacques
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development