SOLR-7381: Usages of Executors#newFixedThreadPool, #newSingleThreadExecutor, #newCachedThreadPool as well as ThreadPoolExecutor directly is now forbidden in Solr

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1673372 13f79535-47bb-0310-9956-ffa450edef68

lucene/tools/forbiddenApis/solr.txt

@@ -13,13 +13,17 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
-@defaultMessage Spawns threads without MDC logging context; use Solr's ExecutorUtil methods to create thread pools
+@defaultMessage Spawns threads without MDC logging context; use ExecutorUtil.newMDCAwareFixedThreadPool instead
 java.util.concurrent.Executors#newFixedThreadPool(int,java.util.concurrent.ThreadFactory)
+
+@defaultMessage Spawns threads without MDC logging context; use ExecutorUtil.newMDCAwareSingleThreadExecutor instead
 java.util.concurrent.Executors#newSingleThreadExecutor(java.util.concurrent.ThreadFactory)
+
+@defaultMessage Spawns threads without MDC logging context; use ExecutorUtil.newMDCAwareCachedThreadPool instead
 java.util.concurrent.Executors#newCachedThreadPool(java.util.concurrent.ThreadFactory)
 
-### TODO - Suppress these for all classes inside Solr except for ExecutorUtil.MDCAwareThreadPoolExecutor
+@defaultMessage Use ExecutorUtil.MDCAwareThreadPoolExecutor instead of ThreadPoolExecutor
-#java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.ThreadFactory,java.util.concurrent.RejectedExecutionHandler)
+java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.ThreadFactory,java.util.concurrent.RejectedExecutionHandler)
-#java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue)
+java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue)
-#java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.ThreadFactory)
+java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.ThreadFactory)
-#java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.RejectedExecutionHandler)
+java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.RejectedExecutionHandler)

solr/CHANGES.txt

@@ -127,7 +127,9 @@ Other Changes
 * SOLR-7371: Make DocSet implement Accountable to estimate memory usage. (yonik, shalin)
 
 * SOLR-7381: Improve logging by adding node name in MDC in SolrCloud mode and adding MDC to
-  all thread pools. (shalin)
+  all thread pools. A new MDCAwareThreadPoolExecutor is introduced and usages of
+  Executors#newFixedThreadPool, #newSingleThreadExecutor, #newCachedThreadPool as well as
+  ThreadPoolExecutor directly is now forbidden in Solr. (shalin)
 
 * SOLR-7384: Fix spurious failures in FullSolrCloudDistribCmdsTest. (shalin)
 

solr/contrib/map-reduce/src/java/org/apache/solr/hadoop/BatchWriter.java

@@ -22,6 +22,7 @@ import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
 import org.apache.solr.client.solrj.response.UpdateResponse;
 import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.util.ExecutorUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -161,7 +162,7 @@ class BatchWriter {
 
     // we need to obtain the settings before the constructor
     if (writerThreads != 0) {
-      batchPool = new ThreadPoolExecutor(writerThreads, writerThreads, 5,
+      batchPool = new ExecutorUtil.MDCAwareThreadPoolExecutor(writerThreads, writerThreads, 5,
           TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(queueSize),
           new ThreadPoolExecutor.CallerRunsPolicy());
     } else { // single threaded case

solr/contrib/map-reduce/src/java/org/apache/solr/hadoop/GoLive.java

@@ -21,6 +21,7 @@ import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.common.util.ExecutorUtil;
 import org.apache.solr.hadoop.MapReduceIndexerTool.Options;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -54,7 +55,7 @@ class GoLive {
     boolean success = false;
     long start = System.nanoTime();
     int concurrentMerges = options.goLiveThreads;
-    ThreadPoolExecutor executor = new ThreadPoolExecutor(concurrentMerges,
+    ThreadPoolExecutor executor = new ExecutorUtil.MDCAwareThreadPoolExecutor(concurrentMerges,
         concurrentMerges, 1, TimeUnit.SECONDS,
         new LinkedBlockingQueue<Runnable>());
     

solr/solrj/build.xml

@@ -20,6 +20,11 @@
 
   <property name="test.lib.dir" location="test-lib"/>
 
+  <!-- Uses ThreadPoolExecutor constructors directly -->
+  <property name="forbidden-base-excludes" value="
+    org/apache/solr/common/util/ExecutorUtil$MDCAwareThreadPoolExecutor.class
+  "/>
+
   <!-- violates the servlet-api restrictions, but it is safe to do so in this test: -->
   <property name="forbidden-tests-excludes" value="
     org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest$DebugServlet.class