Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Impala 2.9.0
    • Fix Version/s: Impala 2.11.0
    • Component/s: Backend
    • Labels:
      None
    • Epic Color:
      ghx-label-9

      Description

      Kudu's util libraries declare quite a few flags, some of which are irrelevant to Impala (as they exist in code that isn't actually used). If possible, we should figure out a way to suppress them from --help and /varz to avoid user confusion.

        Activity

        Show
        sailesh Sailesh Mukil added a comment - Commit in: https://github.com/apache/incubator-impala/commit/ab0bc9eedf1b871a32c25507164101f1d30694db
        Hide
        sailesh Sailesh Mukil added a comment -

        Michael Ho It looks like you need to include the header file and make use of some API from that header.

        I just added this line within a random function inside data-stream-sender.cc:

          kudu::rpc::MessengerBuilder bld("impala-server");
        

        And now I see all the flags from messenger.cc in the log file.

        From this behavior, I can speculate that the compiler doesn't run macros in files that aren't actively used in the code.

        Show
        sailesh Sailesh Mukil added a comment - Michael Ho It looks like you need to include the header file and make use of some API from that header. I just added this line within a random function inside data-stream-sender.cc: kudu::rpc::MessengerBuilder bld( "impala-server" ); And now I see all the flags from messenger.cc in the log file. From this behavior, I can speculate that the compiler doesn't run macros in files that aren't actively used in the code.
        Hide
        kwho Michael Ho added a comment -

        Sailesh Mukil, I cannot quite reproduce the same result by just including the header file. I will check with you offline to understand what's going on. The observation made above still doesn't quite explain why the flags are showing up (or not). May be I am misunderstanding it ?

        Show
        kwho Michael Ho added a comment - Sailesh Mukil , I cannot quite reproduce the same result by just including the header file. I will check with you offline to understand what's going on. The observation made above still doesn't quite explain why the flags are showing up (or not). May be I am misunderstanding it ?
        Hide
        sailesh Sailesh Mukil added a comment -

        Michael Ho Dan Hecht. I looked over the flags that show up in /varz and in the logs and it seems like all the Kudu gflags do show up as long as the Kudu file it belongs to is included by an Impala file. I did this by listing all the flags that show up in be/src/kudu/ and diff-ing against the flags that show up in (be/src/* - be/src/kudu/).

        For eg: We see all the flags from kudu/rpc/messenger.cc since we include kudu/rpc/messenger.h in our Impala files. On the other hand, we don't see flags from kudu/util/spinlock_profiling.cc since kudu/util/spinlock_profiling.h isn't included in any of the Impala files.

        I think that behavior is expected. So my take is that we can go ahead with Henry's above suggestion of marking the flags DEFINE_*_hidden(). I'll put out a patch for that shortly.

        Show
        sailesh Sailesh Mukil added a comment - Michael Ho Dan Hecht . I looked over the flags that show up in /varz and in the logs and it seems like all the Kudu gflags do show up as long as the Kudu file it belongs to is included by an Impala file . I did this by listing all the flags that show up in be/src/kudu/ and diff-ing against the flags that show up in (be/src/* - be/src/kudu/). For eg: We see all the flags from kudu/rpc/messenger.cc since we include kudu/rpc/messenger.h in our Impala files. On the other hand, we don't see flags from kudu/util/spinlock_profiling.cc since kudu/util/spinlock_profiling.h isn't included in any of the Impala files. I think that behavior is expected. So my take is that we can go ahead with Henry's above suggestion of marking the flags DEFINE_*_hidden(). I'll put out a patch for that shortly.
        Hide
        kwho Michael Ho added a comment -

        Actually, the gflags declared in Kudu library are already hidden but it will show up if we do a DECLARE_*(flag_name) in the Impala code. We want to understand the reason for why they are hidden by default.

        Show
        kwho Michael Ho added a comment - Actually, the gflags declared in Kudu library are already hidden but it will show up if we do a DECLARE_*(flag_name) in the Impala code. We want to understand the reason for why they are hidden by default.
        Show
        henryr Henry Robinson added a comment - Fixed in https://github.com/apache/incubator-impala/commit/d1910a39fcc50ce211b95c3552c0c90b4bc37bbd which brings in this gflags commit: https://github.com/henryr/gflags/commit/9ae8eae9a1b6162026854a5266d4ee1427c6d168
        Hide
        henryr Henry Robinson added a comment -

        See https://github.com/henryr/gflags/commit/39909222361957c8d3203d9b7ced1006bad39d59 - this works and passes new unittests. I've tested it with Impala and hidden flags correctly don't show up in user-facing lists.

        For Kudu, rather than try and do some fancy macro-reimplementation trick, I think it's best just to find-and-replace the flag definitions and make them hidden. We can selectively re-publish ones that we want users to see.

        Show
        henryr Henry Robinson added a comment - See https://github.com/henryr/gflags/commit/39909222361957c8d3203d9b7ced1006bad39d59 - this works and passes new unittests. I've tested it with Impala and hidden flags correctly don't show up in user-facing lists. For Kudu, rather than try and do some fancy macro-reimplementation trick, I think it's best just to find-and-replace the flag definitions and make them hidden. We can selectively re-publish ones that we want users to see.
        Hide
        henryr Henry Robinson added a comment -

        The best way I can think of to do this is to patch gflags to have DEFINE_int32_hidden() etc. Flags defined that way will be removed from -help and -varz.

        Show
        henryr Henry Robinson added a comment - The best way I can think of to do this is to patch gflags to have DEFINE_int32_hidden() etc. Flags defined that way will be removed from - help and -varz .

          People

          • Assignee:
            sailesh Sailesh Mukil
            Reporter:
            henryr Henry Robinson
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development