Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-8782

Improve SolrJ Collections async API

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.1
    • Component/s: None
    • Labels:
      None

      Description

      The async collections API is a bit difficult to use at the moment:

      • you need to create your own async ID and add it to the request
      • you then need to create a different request and poll to find out when you get a response
      • the implementation requires some complex generics just to allow a chained .setAsync() method

      I think we can improve this.

      1. SOLR-8782.patch
        48 kB
        Alan Woodward
      2. SOLR-8782.patch
        57 kB
        Alan Woodward

        Activity

        Hide
        romseygeek Alan Woodward added a comment -

        Patch with a suggested implementation.

        • instead of .setAsync(), we add a processAsync() method, that returns a generated async id (you can optionally pass in your own if you have pre-generated ids for somer reason)
        • we also add a processAndWait() method that handles polling for completion or failure states

        This patch applies on top of the patch for SOLR-8765. It needs javadocs, but all tests are passing, and you can see how much simpler the API is from the test changes - lots of lines removed.

        Show
        romseygeek Alan Woodward added a comment - Patch with a suggested implementation. instead of .setAsync(), we add a processAsync() method, that returns a generated async id (you can optionally pass in your own if you have pre-generated ids for somer reason) we also add a processAndWait() method that handles polling for completion or failure states This patch applies on top of the patch for SOLR-8765 . It needs javadocs, but all tests are passing, and you can see how much simpler the API is from the test changes - lots of lines removed.
        Hide
        varunthacker Varun Thacker added a comment -

        Hi Alan,

        +1 for this change. Seems like a good usability improvement.

        Just thinking aloud, would processAsyncAndWait be a better name for processAndWait ? Maybe adding Javadocs and keeping it processAndWait is good enough?

        Show
        varunthacker Varun Thacker added a comment - Hi Alan, +1 for this change. Seems like a good usability improvement. Just thinking aloud, would processAsyncAndWait be a better name for processAndWait ? Maybe adding Javadocs and keeping it processAndWait is good enough?
        Hide
        romseygeek Alan Woodward added a comment -

        SOLR-8765 has missed the 6.0 cutoff, so here's an updated patch for 6.1. Includes javadocs this time. I've also moved CollectionsAPIAsyncDistributedZkTest.java to extend SolrCloudTestBase, which has sped it up by 20s.

        Show
        romseygeek Alan Woodward added a comment - SOLR-8765 has missed the 6.0 cutoff, so here's an updated patch for 6.1. Includes javadocs this time. I've also moved CollectionsAPIAsyncDistributedZkTest.java to extend SolrCloudTestBase, which has sped it up by 20s.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 5b7be9d16abf72052910a63e6d79debd8af5a7c1 in lucene-solr's branch refs/heads/master from Alan Woodward
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5b7be9d ]

        SOLR-8782: Improve async collections API

        Show
        jira-bot ASF subversion and git services added a comment - Commit 5b7be9d16abf72052910a63e6d79debd8af5a7c1 in lucene-solr's branch refs/heads/master from Alan Woodward [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5b7be9d ] SOLR-8782 : Improve async collections API
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 406a1635c11919cbd167eb5ea87461472dbc8d53 in lucene-solr's branch refs/heads/master from Alan Woodward
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=406a163 ]

        Merge branch 'SOLR-8782'

        Show
        jira-bot ASF subversion and git services added a comment - Commit 406a1635c11919cbd167eb5ea87461472dbc8d53 in lucene-solr's branch refs/heads/master from Alan Woodward [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=406a163 ] Merge branch ' SOLR-8782 '
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 761618727d7604b09ec4ecc68d65689c888311f1 in lucene-solr's branch refs/heads/branch_6x from Alan Woodward
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7616187 ]

        SOLR-8782: Improve async collections API

        Show
        jira-bot ASF subversion and git services added a comment - Commit 761618727d7604b09ec4ecc68d65689c888311f1 in lucene-solr's branch refs/heads/branch_6x from Alan Woodward [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7616187 ] SOLR-8782 : Improve async collections API
        Hide
        romseygeek Alan Woodward added a comment -

        Thanks for the review Varun!

        Show
        romseygeek Alan Woodward added a comment - Thanks for the review Varun!
        Hide
        dsmiley David Smiley added a comment -

        I see that setAsyncId is deprecated, yet it is also abstract and most (all?) of the implementations have an identical (and obvious) implementation. What is the wisdom in that? I'd prefer to see it not abstract.

        Show
        dsmiley David Smiley added a comment - I see that setAsyncId is deprecated, yet it is also abstract and most (all?) of the implementations have an identical (and obvious) implementation. What is the wisdom in that? I'd prefer to see it not abstract.
        Hide
        romseygeek Alan Woodward added a comment -

        Yes, it's a bit of a pain. It's because the return type of all the .setAsyncId methods are different. In trunk, once .setAsyncId is removed and all async calls are made via processAndWait(), we can move the identical methods into the Async base class. For now they're left as abstract to ensure that they get implemented properly.

        Show
        romseygeek Alan Woodward added a comment - Yes, it's a bit of a pain. It's because the return type of all the .setAsyncId methods are different. In trunk, once .setAsyncId is removed and all async calls are made via processAndWait(), we can move the identical methods into the Async base class. For now they're left as abstract to ensure that they get implemented properly.
        Hide
        shalinmangar Shalin Shekhar Mangar added a comment -

        I'm curious if you explored just returning a Future instead of implementing processAndWait methods?

        Show
        shalinmangar Shalin Shekhar Mangar added a comment - I'm curious if you explored just returning a Future instead of implementing processAndWait methods?
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 7be7e8beb965714dd1fb1b85f711e9c8a882d088 in lucene-solr's branch refs/heads/branch_6x from Chris Hostetter (Unused)
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7be7e8b ]

        CHANGES.txt corrections - new features go in the New Features section (SOLR-8782, SOLR-8765, SOLR-8842)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 7be7e8beb965714dd1fb1b85f711e9c8a882d088 in lucene-solr's branch refs/heads/branch_6x from Chris Hostetter (Unused) [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7be7e8b ] CHANGES.txt corrections - new features go in the New Features section ( SOLR-8782 , SOLR-8765 , SOLR-8842 )
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 423ec098504836ccd9b6e742a5b93c7b40cb0aa3 in lucene-solr's branch refs/heads/master from Chris Hostetter (Unused)
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=423ec09 ]

        CHANGES.txt corrections - new features go in the New Features section (SOLR-8782, SOLR-8765, SOLR-8842)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 423ec098504836ccd9b6e742a5b93c7b40cb0aa3 in lucene-solr's branch refs/heads/master from Chris Hostetter (Unused) [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=423ec09 ] CHANGES.txt corrections - new features go in the New Features section ( SOLR-8782 , SOLR-8765 , SOLR-8842 )
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 5856dcf5082b2016f94c63cf697fa9f5a44ab303 in lucene-solr's branch refs/heads/branch_6_0 from Steve Rowe
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5856dcf ]

        SOLR-8983: backport to branch_6_0 the required new cloud-minimal configset, originally committed on branch_6x under SOLR-8782

        Show
        jira-bot ASF subversion and git services added a comment - Commit 5856dcf5082b2016f94c63cf697fa9f5a44ab303 in lucene-solr's branch refs/heads/branch_6_0 from Steve Rowe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5856dcf ] SOLR-8983 : backport to branch_6_0 the required new cloud-minimal configset, originally committed on branch_6x under SOLR-8782

          People

          • Assignee:
            romseygeek Alan Woodward
            Reporter:
            romseygeek Alan Woodward
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development