dsmiley commented on code in PR #3477:
URL: https://github.com/apache/solr/pull/3477#discussion_r2280658678


##########
solr/core/src/test/org/apache/solr/update/SolrIndexFingerprintTest.java:
##########
@@ -28,29 +29,21 @@ public class SolrIndexFingerprintTest extends 
SolrTestCaseJ4 {
 
   @BeforeClass
   public static void beforeTests() throws Exception {
-    initCore("solrconfig.xml", "schema.xml");
+    initCore("solrconfig-tiny-segments.xml", "schema.xml");
   }
 
   @Test
   public void testSequentialVsParallelFingerprint() throws Exception {
     long maxVersion = Long.MAX_VALUE;
     SolrCore core = h.getCore();
 
-    // Create a set of 3 segments
-    assertU(adoc("id", "101"));
-    assertU(adoc("id", "102"));
-    assertU(adoc("id", "103"));
-    assertU(commit());
-
-    assertU(adoc("id", "104"));
-    assertU(adoc("id", "105"));
-    assertU(adoc("id", "106"));
-    assertU(commit());
-
-    assertU(adoc("id", "107"));
-    assertU(adoc("id", "108"));
-    assertU(adoc("id", "109"));
-    assertU(commit());
+    // Create a set of 500 segments (to catch race conditions, i.e.SOLR-17863)

Review Comment:
   Let's not repeat a constant in the code with your comment.  I suggest the 
word "many".  There are a number of tests that, in this situation, will pick a 
random number and then also multiply by 
org.apache.lucene.tests.util.LuceneTestCase.RANDOM_MULTIPLIER so nightly pushes 
it harder.  Not saying you have to do that!  How long does this test take at 
500?



##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -270,8 +271,8 @@ public Date getStartTimeStamp() {
     return startTime;
   }
 
-  private final Map<IndexReader.CacheKey, IndexFingerprint> 
perSegmentFingerprintCache =
-      new WeakHashMap<>();
+  private final Cache<IndexReader.CacheKey, IndexFingerprint> 
perSegmentFingerprintCache =

Review Comment:
   I know this is strictly unrelated but I **hate** that we have a field 
defined here, shoved inbetween two methods (whose methods are slightly related 
and neither have *anything* to do with this field).  Sloppy as hell; it grates 
at my sensibilities.  I don't even like it in this class TBH but whatever.  Can 
you please move it?  Maybe below ConfigSet.



##########
solr/core/src/test-files/solr/collection1/conf/solrconfig-tiny-segments.xml:
##########
@@ -0,0 +1,32 @@
+<?xml version="1.0" ?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<config>
+  <luceneMatchVersion>${tests.luceneMatchVersion:LATEST}</luceneMatchVersion>
+  <directoryFactory name="DirectoryFactory" 
class="${solr.directoryFactory:solr.MockDirectoryFactory}"/>
+  <schemaFactory class="ClassicIndexSchemaFactory"/>
+
+  <indexConfig>
+    <useCompoundFile>${useCompoundFile:false}</useCompoundFile>
+    <mergePolicyFactory class="org.apache.solr.index.TieredMergePolicyFactory">
+      <double name="maxMergedSegmentMB">0.0001</double>

Review Comment:
   if your goal is lots of segments, the textbook Lucene way to do this is 
NoMergePolicy.  It explicitly conveys the goal whereas this configuration is 
less apparent as to the intent.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to