Uploaded image for project: 'IMPALA'
  1. IMPALA
  2. IMPALA-4246

SleepForMs() utility function has undefined behavior for > 1s

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: Impala 2.6.0
    • Fix Version/s: Impala 2.8.0
    • Component/s: Backend
    • Labels:
      None

      Description

      Our SleepForMs() function relies on usleep() which sleeps for n microseconds. However, the manpage for usleep() specifies that this may not work for values > 1000000 us (or 1s).

      SYNOPSIS
      ...
             int usleep(useconds_t usec);
      ...
      
      ERRORS
      ...
             EINVAL usec is not smaller than 1000000.  (On systems where that is considered an error.)
      

      We also do not check the return value of usleep(). This means that sleeps can happen for undefined periods of time depending on the system.

      Found this issue after setting a high value for the "kerberos_reinit_interval" flag and noticed that there were very frequent kinit's happening.

        Activity

        Hide
        henryr Henry Robinson added a comment -

        Does usleep(1000001) return an error on your machine? (Note "on systems where that is considered an error").

        Regardless, we should use std::this_thread::sleep_for(std::chrono::milliseconds( x )); instead.

        Show
        henryr Henry Robinson added a comment - Does usleep(1000001) return an error on your machine? (Note "on systems where that is considered an error"). Regardless, we should use std::this_thread::sleep_for(std::chrono::milliseconds( x )); instead.
        Hide
        sailesh Sailesh Mukil added a comment -

        On all machines I've seen so far, usleep(3600000000) works fine (i.e. sleep for 60 minutes), even though it's way above the 1000000 limitation. This problem was found when trying to sleep for 12 hours (720 minutes).

        In any case, I'll switch to using sleep_for().

        Show
        sailesh Sailesh Mukil added a comment - On all machines I've seen so far, usleep(3600000000) works fine (i.e. sleep for 60 minutes), even though it's way above the 1000000 limitation. This problem was found when trying to sleep for 12 hours (720 minutes). In any case, I'll switch to using sleep_for().
        Hide
        henryr Henry Robinson added a comment -

        So you have seen the problem when sleeping for 12 hours? Presumably there's some overflow: 60 minutes in us < 2 ** 32 < 120 minutes in us

        Show
        henryr Henry Robinson added a comment - So you have seen the problem when sleeping for 12 hours? Presumably there's some overflow: 60 minutes in us < 2 ** 32 < 120 minutes in us
        Hide
        sailesh Sailesh Mukil added a comment -

        Yes I have seen the problem with 12 hours. An overflow is probably what's happening.

        Show
        sailesh Sailesh Mukil added a comment - Yes I have seen the problem with 12 hours. An overflow is probably what's happening.
        Hide
        jyu@cloudera.com Juan Yu added a comment -

        Sailesh Mukil should this one be closed now?

        Show
        jyu@cloudera.com Juan Yu added a comment - Sailesh Mukil should this one be closed now?
        Show
        sailesh Sailesh Mukil added a comment - Commit in: https://github.com/apache/incubator-impala/commit/3be113cb9fd6460a80b8198a50f3619c4b9539a2

          People

          • Assignee:
            sailesh Sailesh Mukil
            Reporter:
            sailesh Sailesh Mukil
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development