Solr
  1. Solr
  2. SOLR-7741

SolrIndexerConfig.toMap missing fields

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 6.0
    • Fix Version/s: 5.3
    • Component/s: None
    • Labels:
      None

      Description

      From the mailing list:

      Mike Drob wrote:

      I was looking through code for unrelated reasons and this line stuck out to me:

      https://github.com/apache/lucene-solr/blob/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java#L180

      if(mergeSchedulerInfo != null) m.put("mergeScheduler",mergeSchedulerInfo.toMap()); if(mergePolicyInfo != null) m.put("mergeScheduler",mergePolicyInfo.toMap());

      Are they both supposed to be using the "mergeScheduler" key? If not, happy to open a JIRA and provide the one-line patch.

      Christine Poerschke wrote:

      Follow-on question, also re: SolrIndexConfig's toMap method. The constructor reads "/mergedSegmentWarmer" and "/infoStream" and "/infoStream/@file" elements but the toMap method does not write them.

      Would that be intended or is it maybe an unintended omission? If intended would be great to have a comment in toMap to clarify why/which elements are skipped, so that if/when a new element is added its easier to determine if toMap should write the new element also.

      1. SOLR-7741.patch
        6 kB
        Christine Poerschke
      2. SOLR-7741.patch
        4 kB
        Mike Drob

        Issue Links

          Activity

          Hide
          Mike Drob added a comment -

          Christine Poerschke - I hope you don't mind that I added your missing fields as well. Please review if this is what you had meant for.

          Patch for trunk attached that add the missing fields, renames the conflicting mergePolicy field, and adds a test.

          Show
          Mike Drob added a comment - Christine Poerschke - I hope you don't mind that I added your missing fields as well. Please review if this is what you had meant for. Patch for trunk attached that add the missing fields, renames the conflicting mergePolicy field, and adds a test.
          Hide
          Christine Poerschke added a comment - - edited

          Hi Mike Drob - thanks for including my missing fields in your patch.

          Am attaching a patch that extends your patch:

          • "useCompoundFile" was also missing in SolrIndexerConfig.toMap (i only noticed that today and so added it)
          • the SolrIndexConfigTest.testToMap started by your patch now randomly chooses between solrconfig-warmer.xml and solrconfig-tieredmergepolicy.xml SolrConfig and it checks presence+type for non-Map fields also
          Show
          Christine Poerschke added a comment - - edited Hi Mike Drob - thanks for including my missing fields in your patch. Am attaching a patch that extends your patch: "useCompoundFile" was also missing in SolrIndexerConfig.toMap (i only noticed that today and so added it) the SolrIndexConfigTest.testToMap started by your patch now randomly chooses between solrconfig-warmer.xml and solrconfig-tieredmergepolicy.xml SolrConfig and it checks presence+type for non-Map fields also
          Hide
          Mike Drob added a comment -

          +1

          Show
          Mike Drob added a comment - +1
          Hide
          ASF subversion and git services added a comment -

          Commit 1689041 from Ramkumar Aiyengar in branch 'dev/trunk'
          [ https://svn.apache.org/r1689041 ]

          SOLR-7741: Add missing fields to SolrIndexerConfig.toMap

          Show
          ASF subversion and git services added a comment - Commit 1689041 from Ramkumar Aiyengar in branch 'dev/trunk' [ https://svn.apache.org/r1689041 ] SOLR-7741 : Add missing fields to SolrIndexerConfig.toMap
          Hide
          ASF subversion and git services added a comment -

          Commit 1689042 from Ramkumar Aiyengar in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1689042 ]

          SOLR-7741: Add missing fields to SolrIndexerConfig.toMap

          Show
          ASF subversion and git services added a comment - Commit 1689042 from Ramkumar Aiyengar in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1689042 ] SOLR-7741 : Add missing fields to SolrIndexerConfig.toMap
          Hide
          Ramkumar Aiyengar added a comment -

          Thanks Mike, Christine.

          Show
          Ramkumar Aiyengar added a comment - Thanks Mike, Christine.
          Hide
          Shalin Shekhar Mangar added a comment -

          Bulk close for 5.3.0 release

          Show
          Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

            People

            • Assignee:
              Ramkumar Aiyengar
              Reporter:
              Mike Drob
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development