Solr
  1. Solr
  2. SOLR-3883

distributed indexing forwards non-applicable request params

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0, 6.0
    • Component/s: SolrCloud
    • Labels:
      None

      Description

      Including all of the original request params can cause really undesirable stuff to happen.

      Reported by Dan Sutton:
      http://find.searchhub.org/document/c21c27f20e115ce9

        Activity

        Hide
        Mark Miller added a comment -

        Per had brought this up as a possible issue in the past, but it was tied up in an issue with a lot of other stuff.

        Was trying to avoid having to guess everything that should go, but looks like the reverse may be worse.

        Show
        Mark Miller added a comment - Per had brought this up as a possible issue in the past, but it was tied up in an issue with a lot of other stuff. Was trying to avoid having to guess everything that should go, but looks like the reverse may be worse.
        Hide
        Yonik Seeley added a comment -

        First guess is that it's probably safer/easier to add params we know we need rather than remove params that could do things we don't want? It just seems like that as we add new methods of creating documents in the future, "dangerous" params could proliferate. Seems like it may be easier to enumerate those params that are necessary/desirable for updates.

        Brainstorming off the top of my head:

        • Params for updates: commitWithin, NOW?
        • Params for commits: softCommit, waitSearcher?
        Show
        Yonik Seeley added a comment - First guess is that it's probably safer/easier to add params we know we need rather than remove params that could do things we don't want? It just seems like that as we add new methods of creating documents in the future, "dangerous" params could proliferate. Seems like it may be easier to enumerate those params that are necessary/desirable for updates. Brainstorming off the top of my head: Params for updates: commitWithin, NOW? Params for commits: softCommit, waitSearcher?
        Hide
        Mark Miller added a comment -

        Params for updates: commitWithin, NOW? Params for commits: softCommit, waitSearcher?

        I've got to brush up on the NOW param...

        I think the commit params are all fine - we build a command from the orig request and then pass that on.

        We do need to do UpdateParams.UPDATE_CHAIN, but I am not 100% on what else yet.

        Show
        Mark Miller added a comment - Params for updates: commitWithin, NOW? Params for commits: softCommit, waitSearcher? I've got to brush up on the NOW param... I think the commit params are all fine - we build a command from the orig request and then pass that on. We do need to do UpdateParams.UPDATE_CHAIN, but I am not 100% on what else yet.
        Hide
        Mark Miller added a comment -

        Here is an initial patch that only pass UPDATE_CHAIN - no handling of NOW.

        Show
        Mark Miller added a comment - Here is an initial patch that only pass UPDATE_CHAIN - no handling of NOW.
        Hide
        Yonik Seeley added a comment -

        Yeah, I guess commit params are already handled within the command - no need for request params.
        Is this also the case for commitWithin on adds, deletes, deleteByQuery?

        Show
        Yonik Seeley added a comment - Yeah, I guess commit params are already handled within the command - no need for request params. Is this also the case for commitWithin on adds, deletes, deleteByQuery?
        Hide
        Mark Miller added a comment -

        Is this also the case for commitWithin on adds, deletes, deleteByQuery?

        I suppose not - this just covers the commit case. It also covers commitWithin from xml or whatever, but it seems we do also have to worry about the case where it's just a request params on an add or delete.

        Show
        Mark Miller added a comment - Is this also the case for commitWithin on adds, deletes, deleteByQuery? I suppose not - this just covers the commit case. It also covers commitWithin from xml or whatever, but it seems we do also have to worry about the case where it's just a request params on an add or delete.
        Hide
        Mark Miller added a comment -

        I suppose not

        I take that back - looking further, it seems all ways commitWithin can come in, it gets set on the update command, and our update command distributor then respects sending them on.

        Show
        Mark Miller added a comment - I suppose not I take that back - looking further, it seems all ways commitWithin can come in, it gets set on the update command, and our update command distributor then respects sending them on.
        Hide
        Mark Miller added a comment -

        I think NOW could probably be considered it's own separate issue. I think we should get this change in. Anything I still may be missing?

        Show
        Mark Miller added a comment - I think NOW could probably be considered it's own separate issue. I think we should get this change in. Anything I still may be missing?
        Hide
        Mark Miller added a comment -

        I've committed the work so far.

        Show
        Mark Miller added a comment - I've committed the work so far.
        Hide
        Mark Miller added a comment -

        Okay, barring further feedback from yonik, I'm calling this resolved. We can open a new issue to consider how we want to handle NOW values with distrib indexing.

        Show
        Mark Miller added a comment - Okay, barring further feedback from yonik, I'm calling this resolved. We can open a new issue to consider how we want to handle NOW values with distrib indexing.
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Mark Robert Miller
        http://svn.apache.org/viewvc?view=revision&revision=1390274

        SOLR-3883: Distributed indexing forwards non-applicable request params.

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Mark Robert Miller http://svn.apache.org/viewvc?view=revision&revision=1390274 SOLR-3883 : Distributed indexing forwards non-applicable request params.
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Mark Robert Miller
        http://svn.apache.org/viewvc?view=revision&revision=1390274

        SOLR-3883: Distributed indexing forwards non-applicable request params.

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Mark Robert Miller http://svn.apache.org/viewvc?view=revision&revision=1390274 SOLR-3883 : Distributed indexing forwards non-applicable request params.
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Mark Miller
            Reporter:
            Yonik Seeley
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development