HUE-9100 [beeswax] Multi-session support

Review Request #14605 — Created Dec. 9, 2019 and submitted

jgauthier
hue
master
HUE-9100
hue
jgauthier, johan, ranade, romain, weixia, yingc
commit 99f081a33cfe8dda9a1ed6124c0a74a9aecf7c01
Author: Jean-Francois Desjeans Gauthier <jf.desjeans.gauthier@gmail.com>
Date:   Mon Dec 9 16:48:38 2019 -0800

    HUE-9100 [beeswax] Multi-session support

:100644 100644 6d055e5af4... b62d430322... M	apps/beeswax/src/beeswax/models.py
:100644 100644 d19dc173db... 7c2ac67ae7... M	apps/beeswax/src/beeswax/server/dbms.py
:100644 100644 f9869a185e... 195e7ea5b3... M	apps/beeswax/src/beeswax/server/hive_server2_lib.py
:100644 100644 a606957928... 3c9ce78949... M	desktop/core/src/desktop/js/apps/notebook/snippet.js
:100644 100644 ece0dad70f... 9ead6dfa90... M	desktop/core/src/desktop/lib/python_util.py
:100644 100644 ca310e4379... c86fc07db3... M	desktop/core/src/desktop/lib/python_util_test.py
:100644 100644 05b1b9c2bc... 96ed540994... M	desktop/libs/notebook/src/notebook/api.py
:100644 100644 da53b4c4e6... 82f2b54713... M	desktop/libs/notebook/src/notebook/connectors/hiveserver2.py

Single session
1 editor. Create, execute, close, execute
2 editor. Close session in editor 1 & execute. Execute in editor 2
1 shared editor: execute, close execute
2 shared editor: Close session in editor 1 & execute. Execute in editor 2

Multi-session
1 editor. Create, execute, close, execute
2 editor. Close session in editor 1 & execute. Execute in editor 2

Create table from file

More testing needed

  • 0
  • 0
  • 18
  • 1
  • 19
Description From Last Updated
romain
  1. Nice progress!

    But a bunch of comments, mostly about how we could maybe refactor a bit to keep it as simple/contained as possible.

    Note: the 3 cases in the create session for loop are probably perfect candidate for unit tests with mocks?
    https://github.com/cloudera/hue/tree/master/apps/beeswax/src/beeswax/server

    (that way we can stay confident/test easily the logic and automate it)

  2. Any chance to avoid a loop with continues and do a series of if / else?

    res, session = call_return_result_and_session util()
    if res is None:
    ...
    elif:
    ..
    else:
    ...

    return

  3. Now?

    _call_return_result_and_session --> _call_and_return_result

  4. apps/beeswax/src/beeswax/server/hive_server2_lib.py (Diff revision 1)
     
     
     
     
     

    Avoid duplication with a util?

    with_multiple_sessions()

    or

    has_multiple_sessions()

    (to also keep in mind how to handle Hive with multi sessions and Impala with single sessions, when both are configured in Hue)

  5. Not used anywhere?

  6. Using the same util? (maybe better to move to the conf that way there is no logic duplication)

  7. Could we avoid any Thrift here? (it is in the Notebook lib so should be agnostic, so better to move the Thrift logic in the hiveserver2 connector)

  8. 
      
jgauthier
jgauthier
jgauthier
  1. 
      
  2. Will add some tests

  3. 
      
jgauthier
romain
  1. This is nice and very much contained now, great stuff!

    I just put a bunch of nits, still "digesting" the core logic a little bit more, but not forseeing any major changes.

  2. apps/beeswax/src/beeswax/conf.py (Diff revision 4)
     
     

    Wondering:

    Would make it sense to have the same in impala/conf.py (like we do for some other params, that way we can differentiate both?)

    (and maybe 3 for hive, -1 for Impala like today?)

    1. I'm thinking maybe we put it at [editor] and have the hive one overwride for hive?

    2. I think for now, will just do this for hive.

    3. +1 to even hardcode in for only beeswax lib based on the selected interface. We can tweak later when using the new connectors.

  3. apps/beeswax/src/beeswax/conf.py (Diff revision 4)
     
     

    nit: Comment on new line as long?

  4. apps/beeswax/src/beeswax/conf.py (Diff revision 4)
     
     

    dynamic_default to true when 'max_number_of_sessions != 1?

  5. apps/beeswax/src/beeswax/conf.py (Diff revision 4)
     
     

    Update parameters in inis too?

  6. apps/beeswax/src/beeswax/conf.py (Diff revision 4)
     
     

    CLOSE_IDLE_SESSIONS.get() == False --> not CLOSE_IDLE_SESSIONS.get()

  7. apps/beeswax/src/beeswax/conf.py (Diff revision 4)
     
     

    CLOSE_IDLE_SESSIONS.get() == True --> CLOSE_IDLE_SESSIONS.get()

  8. apps/beeswax/src/beeswax/models.py (Diff revision 4)
     
     
     
     
     

    Avoid dup?

    q = q.order_by("-last_used")
    if n > 0:
    q = q[0:n]

  9. Time to remove 'with_multiple_session arg'?

  10. Does it mean it can fails with LLAP? Or it is another corner case?

    LOG.warn() might be useful do avoid silent errors.

    1. Fails in the test and needs correct mocking to get right. I can add warn.

  11. nit, can also make it the same indentation so simpler:

    finish = (
    MAX_NUMBER_OF_SESSIONS.set_for_testing(1),
    CLOSE_IDLE_SESSIONS.set_for_testing(False)
    )

  12. 
      
romain
  1. Bunch of last nits!

    Splitting conf properties between hive/impala would help when we switch to connectors (so that the configs are only specific for each SQL dialect) and leverage the good support of Impala concurrent queries.
    Will have to see with SQL Task Server at some point. Curious to see in practice how it works for slow booting sessions + multi statements that might run in different sessions, but let's iterate after.

    +1 for blog + documentation update :)

  2. apps/beeswax/src/beeswax/conf.py (Diff revision 4)
     
     

    CLOSE_IDLE_SESSIONS --> CLOSE_SESSIONS?

    That way similar name than CLOSE_QUERIES.
    When leaving the notebook, also close session.

    If only about managed queries, maybe just CLOSE_MANAGED_SESSIONS?

  3. apps/beeswax/src/beeswax/conf.py (Diff revision 4)
     
     

    Worth adding a section to document the close/max session properties now?

    https://docs.gethue.com/administrator/administration/reference/#hive-and-impala-queries-life-cycle

    (kind of difficult to know how to use)

  4. 
      
jgauthier
jgauthier
jgauthier
Review request changed

Status: Closed (submitted)

Loading...