Uploaded image for project: 'Apache Drill'
  1. Apache Drill
  2. DRILL-4354

Remove sessions in anonymous (user auth disabled) mode in WebUI server

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.5.0
    • 1.6.0
    • Server
    • None

    Description

      Currently we open anonymous sessions when user auth disabled. These sessions are cleaned up when they expire (controlled by boot config drill.exec.http.session_max_idle_secs). This may lead to unnecessary resource accumulation. This JIRA is to remove anonymous sessions and only have sessions when user authentication is enabled.

      Attachments

        Activity

          githubbot ASF GitHub Bot added a comment -

          GitHub user vkorukanti opened a pull request:

          https://github.com/apache/drill/pull/360

          DRILL-4354: Remove sessions in anonymous (auth disabled) WebUI access

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/vkorukanti/drill DRILL-4354

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/drill/pull/360.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #360


          commit af18fa8c5d0b52a15c2686fe6ed6ba34956ff351
          Author: vkorukanti <venki.korukanti@gmail.com>
          Date: 2016-02-04T21:06:54Z

          DRILL-4353: Add HttpSessionListener to release resources of expired/invalidated sessions

          commit 0ed8fe8056f87d50660f4a8d8c61e3519952fd0f
          Author: vkorukanti <venki.korukanti@gmail.com>
          Date: 2016-02-02T14:48:18Z

          DRILL-4354: Remove sessions in anonymous (auth disabled) WebUI access


          githubbot ASF GitHub Bot added a comment - GitHub user vkorukanti opened a pull request: https://github.com/apache/drill/pull/360 DRILL-4354 : Remove sessions in anonymous (auth disabled) WebUI access You can merge this pull request into a Git repository by running: $ git pull https://github.com/vkorukanti/drill DRILL-4354 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/360.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #360 commit af18fa8c5d0b52a15c2686fe6ed6ba34956ff351 Author: vkorukanti <venki.korukanti@gmail.com> Date: 2016-02-04T21:06:54Z DRILL-4353 : Add HttpSessionListener to release resources of expired/invalidated sessions commit 0ed8fe8056f87d50660f4a8d8c61e3519952fd0f Author: vkorukanti <venki.korukanti@gmail.com> Date: 2016-02-02T14:48:18Z DRILL-4354 : Remove sessions in anonymous (auth disabled) WebUI access
          githubbot ASF GitHub Bot added a comment -

          Github user sudheeshkatkam commented on a diff in the pull request:

          https://github.com/apache/drill/pull/360#discussion_r54200801

          — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java —
          @@ -102,4 +112,34 @@ public void dispose(DrillUserPrincipal principal)

          { // No-Op }

          }
          +
          + // Provider which creates and cleanups DrillUserPrincipal for anonymous (auth disabled) mode
          + public static class AnonDrillUserPrincipalProvider implements Factory<DrillUserPrincipal> {
          + @Inject WorkManager workManager;
          +
          + @RequestScoped
          + @Override
          + public DrillUserPrincipal provide()

          { + return new AnonDrillUserPrincipal(workManager.getContext()); + }

          +
          + @Override
          + public void dispose(DrillUserPrincipal principal) {
          + // If this worked it would have been clean to free the resources here, but there are various scenarios
          — End diff –

          Any [specific tickets](https://java.net/jira/browse/JERSEY/)?

          githubbot ASF GitHub Bot added a comment - Github user sudheeshkatkam commented on a diff in the pull request: https://github.com/apache/drill/pull/360#discussion_r54200801 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java — @@ -102,4 +112,34 @@ public void dispose(DrillUserPrincipal principal) { // No-Op } } + + // Provider which creates and cleanups DrillUserPrincipal for anonymous (auth disabled) mode + public static class AnonDrillUserPrincipalProvider implements Factory<DrillUserPrincipal> { + @Inject WorkManager workManager; + + @RequestScoped + @Override + public DrillUserPrincipal provide() { + return new AnonDrillUserPrincipal(workManager.getContext()); + } + + @Override + public void dispose(DrillUserPrincipal principal) { + // If this worked it would have been clean to free the resources here, but there are various scenarios — End diff – Any [specific tickets] ( https://java.net/jira/browse/JERSEY/)?
          githubbot ASF GitHub Bot added a comment -

          Github user sudheeshkatkam commented on a diff in the pull request:

          https://github.com/apache/drill/pull/360#discussion_r54200836

          — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillUserPrincipal.java —
          @@ -60,13 +63,21 @@ public String getName() {
          }

          /**

          • * @return Return {@link DrillClient} instanced with credentials of this user principal.
            + * @return Return {@link DrillClient}

            instanced with credentials of this user principal. Returned

            {@link DrillClient}

            + * must be returned using

            {@link #recycleDrillClient(DrillClient)}

            for proper resource cleanup.
            */

          • public DrillClient getDrillClient() {
            + public DrillClient getDrillClient() throws IOException { return drillClient; }

          /**
          + *
          — End diff –

          missing?

          githubbot ASF GitHub Bot added a comment - Github user sudheeshkatkam commented on a diff in the pull request: https://github.com/apache/drill/pull/360#discussion_r54200836 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillUserPrincipal.java — @@ -60,13 +63,21 @@ public String getName() { } /** * @return Return {@link DrillClient} instanced with credentials of this user principal. + * @return Return {@link DrillClient} instanced with credentials of this user principal. Returned {@link DrillClient} + * must be returned using {@link #recycleDrillClient(DrillClient)} for proper resource cleanup. */ public DrillClient getDrillClient() { + public DrillClient getDrillClient() throws IOException { return drillClient; } /** + * — End diff – missing?
          githubbot ASF GitHub Bot added a comment -

          Github user sudheeshkatkam commented on the pull request:

          https://github.com/apache/drill/pull/360#issuecomment-189091692

          +1

          I have two minor comments. Looks like DRILL-4353 is already committed.

          githubbot ASF GitHub Bot added a comment - Github user sudheeshkatkam commented on the pull request: https://github.com/apache/drill/pull/360#issuecomment-189091692 +1 I have two minor comments. Looks like DRILL-4353 is already committed.
          githubbot ASF GitHub Bot added a comment -

          Github user vkorukanti commented on a diff in the pull request:

          https://github.com/apache/drill/pull/360#discussion_r54273230

          — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java —
          @@ -102,4 +112,34 @@ public void dispose(DrillUserPrincipal principal)

          { // No-Op }

          }
          +
          + // Provider which creates and cleanups DrillUserPrincipal for anonymous (auth disabled) mode
          + public static class AnonDrillUserPrincipalProvider implements Factory<DrillUserPrincipal> {
          + @Inject WorkManager workManager;
          +
          + @RequestScoped
          + @Override
          + public DrillUserPrincipal provide()

          { + return new AnonDrillUserPrincipal(workManager.getContext()); + }

          +
          + @Override
          + public void dispose(DrillUserPrincipal principal) {
          + // If this worked it would have been clean to free the resources here, but there are various scenarios
          — End diff –

          Original issue: https://java.net/jira/browse/JERSEY-2299. It got fixed in 2.7 (we are on 2.8), but I saw few threads that mention it still reproes and not always called when closing the browser (http://jersey.576304.n2.nabble.com/Factory-dispose-is-not-called-td7583326.html). Didn't find any latest JIRAs. When I tried also, it never called after the request is complete. So thought it is not safe to rely on this method to cleanup resources.

          githubbot ASF GitHub Bot added a comment - Github user vkorukanti commented on a diff in the pull request: https://github.com/apache/drill/pull/360#discussion_r54273230 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java — @@ -102,4 +112,34 @@ public void dispose(DrillUserPrincipal principal) { // No-Op } } + + // Provider which creates and cleanups DrillUserPrincipal for anonymous (auth disabled) mode + public static class AnonDrillUserPrincipalProvider implements Factory<DrillUserPrincipal> { + @Inject WorkManager workManager; + + @RequestScoped + @Override + public DrillUserPrincipal provide() { + return new AnonDrillUserPrincipal(workManager.getContext()); + } + + @Override + public void dispose(DrillUserPrincipal principal) { + // If this worked it would have been clean to free the resources here, but there are various scenarios — End diff – Original issue: https://java.net/jira/browse/JERSEY-2299 . It got fixed in 2.7 (we are on 2.8), but I saw few threads that mention it still reproes and not always called when closing the browser ( http://jersey.576304.n2.nabble.com/Factory-dispose-is-not-called-td7583326.html ). Didn't find any latest JIRAs. When I tried also, it never called after the request is complete. So thought it is not safe to rely on this method to cleanup resources.
          githubbot ASF GitHub Bot added a comment -

          Github user vkorukanti commented on a diff in the pull request:

          https://github.com/apache/drill/pull/360#discussion_r54273249

          — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillUserPrincipal.java —
          @@ -60,13 +63,21 @@ public String getName() {
          }

          /**

          • * @return Return {@link DrillClient} instanced with credentials of this user principal.
            + * @return Return {@link DrillClient}

            instanced with credentials of this user principal. Returned

            {@link DrillClient}

            + * must be returned using

            {@link #recycleDrillClient(DrillClient)}

            for proper resource cleanup.
            */

          • public DrillClient getDrillClient() {
            + public DrillClient getDrillClient() throws IOException { return drillClient; }

          /**
          + *
          — End diff –

          added javadoc.

          githubbot ASF GitHub Bot added a comment - Github user vkorukanti commented on a diff in the pull request: https://github.com/apache/drill/pull/360#discussion_r54273249 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillUserPrincipal.java — @@ -60,13 +63,21 @@ public String getName() { } /** * @return Return {@link DrillClient} instanced with credentials of this user principal. + * @return Return {@link DrillClient} instanced with credentials of this user principal. Returned {@link DrillClient} + * must be returned using {@link #recycleDrillClient(DrillClient)} for proper resource cleanup. */ public DrillClient getDrillClient() { + public DrillClient getDrillClient() throws IOException { return drillClient; } /** + * — End diff – added javadoc.
          githubbot ASF GitHub Bot added a comment -

          Github user vkorukanti closed the pull request at:

          https://github.com/apache/drill/pull/360

          githubbot ASF GitHub Bot added a comment - Github user vkorukanti closed the pull request at: https://github.com/apache/drill/pull/360
          knguyen Krystal added a comment -

          git.commit.id.abbrev=64ab0a8

          disabled authentication in drill-override.conf with drill.exec.http.session_max_idle_secs=10.

          With debug turned on, saw the following session info in the drillbit.log file using drill-1.5:
          2016-03-11 09:55:12,368 [qtp747584933-50] DEBUG o.a.drill.exec.client.DrillClient - Connecting to server <hostname>:31010
          2016-03-11 09:55:23,382 [Scheduler-1866682207] DEBUG o.apache.drill.exec.rpc.BasicClient - Closing client
          2016-03-11 09:55:23,382 [Client-1] INFO o.a.drill.exec.rpc.user.UserClient - Channel closed /<ip>:57195 <--> /<ip>:31010.

          In drill-1.6, the above session info is no longer present for anonymous connections.

          knguyen Krystal added a comment - git.commit.id.abbrev=64ab0a8 disabled authentication in drill-override.conf with drill.exec.http.session_max_idle_secs=10. With debug turned on, saw the following session info in the drillbit.log file using drill-1.5: 2016-03-11 09:55:12,368 [qtp747584933-50] DEBUG o.a.drill.exec.client.DrillClient - Connecting to server <hostname>:31010 2016-03-11 09:55:23,382 [Scheduler-1866682207] DEBUG o.apache.drill.exec.rpc.BasicClient - Closing client 2016-03-11 09:55:23,382 [Client-1] INFO o.a.drill.exec.rpc.user.UserClient - Channel closed /<ip>:57195 <--> /<ip>:31010. In drill-1.6, the above session info is no longer present for anonymous connections.
          knguyen Krystal added a comment -

          Verified.

          knguyen Krystal added a comment - Verified.

          People

            vkorukanti Venki Korukanti
            vkorukanti Venki Korukanti
            Krystal Krystal
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: