You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/09/09 18:43:27 UTC
[lucene-solr] branch reference_impl_dev updated: @825 A little
cleanup and shutdown work.
This is an automated email from the ASF dual-hosted git repository.
markrmiller pushed a commit to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/reference_impl_dev by this push:
new c55655e @825 A little cleanup and shutdown work.
c55655e is described below
commit c55655ea9f8d0fd75af460105bd60e05293b04fc
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Wed Sep 9 13:40:53 2020 -0500
@825 A little cleanup and shutdown work.
---
.../solr/handler/admin/MetricsHistoryHandler.java | 18 ++++++++++++++----
.../apache/solr/metrics/rrd/SolrRrdBackendFactory.java | 12 ++++++++----
.../org/apache/solr/servlet/SolrDispatchFilter.java | 13 ++-----------
.../org/apache/solr/servlet/SolrRequestParsers.java | 9 ++-------
.../src/java/org/apache/solr/util/SafeXMLParsing.java | 2 +-
5 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
index 1e52aba..2fa09b4 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
@@ -39,6 +39,7 @@ import java.util.Set;
import java.util.TimeZone;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -157,6 +158,7 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
private final Map<String, RrdDb> knownDbs = new ConcurrentHashMap<>(16, 0.75f, 4);
private final Overseer overseer;
+ private ScheduledFuture<?> scheduledFuture;
private ScheduledThreadPoolExecutor collectService;
private boolean logMissingCollection = true;
@@ -221,10 +223,10 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
if (enable) {
collectService = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(1,
- new SolrNamedThreadFactory("MetricsHistoryHandler"));
+ new SolrNamedThreadFactory("MetricsHistoryHandler", true));
collectService.setRemoveOnCancelPolicy(true);
collectService.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
- collectService.scheduleWithFixedDelay(() -> collectMetrics(),
+ scheduledFuture = collectService.scheduleWithFixedDelay(() -> collectMetrics(),
timeSource.convertDelay(TimeUnit.SECONDS, collectPeriod, TimeUnit.MILLISECONDS),
timeSource.convertDelay(TimeUnit.SECONDS, collectPeriod, TimeUnit.MILLISECONDS),
TimeUnit.MILLISECONDS);
@@ -617,10 +619,18 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
if (log.isDebugEnabled()) {
log.debug("Closing {}", hashCode());
}
- collectService.shutdownNow();
+
+ if (collectService != null) {
+ collectService.shutdown();
+
+ scheduledFuture.cancel(true);
+ }
+
try (ParWork closer = new ParWork(this)) {
- closer.collect(factory);
closer.collect(knownDbs.values());
+ closer.collect();
+ closer.collect(factory);
+ closer.collect();
closer.collect(collectService);
}
knownDbs.clear();
diff --git a/solr/core/src/java/org/apache/solr/metrics/rrd/SolrRrdBackendFactory.java b/solr/core/src/java/org/apache/solr/metrics/rrd/SolrRrdBackendFactory.java
index 0fa0d9b..f13c508 100644
--- a/solr/core/src/java/org/apache/solr/metrics/rrd/SolrRrdBackendFactory.java
+++ b/solr/core/src/java/org/apache/solr/metrics/rrd/SolrRrdBackendFactory.java
@@ -30,6 +30,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
@@ -84,6 +85,7 @@ public class SolrRrdBackendFactory extends RrdBackendFactory implements SolrClos
private final String collection;
private final int syncPeriod;
private final int idPrefixLength;
+ private final ScheduledFuture<?> scheduledFuture;
private ScheduledThreadPoolExecutor syncService;
private volatile boolean closed = false;
private volatile boolean persistent = true;
@@ -110,10 +112,10 @@ public class SolrRrdBackendFactory extends RrdBackendFactory implements SolrClos
}
this.idPrefixLength = ID_PREFIX.length() + ID_SEP.length();
syncService = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(2,
- new SolrNamedThreadFactory("SolrRrdBackendFactory"));
+ new SolrNamedThreadFactory("SolrRrdBackendFactory", true));
syncService.setRemoveOnCancelPolicy(true);
syncService.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
- syncService.scheduleWithFixedDelay(() -> maybeSyncBackends(),
+ scheduledFuture = syncService.scheduleWithFixedDelay(() -> maybeSyncBackends(),
timeSource.convertDelay(TimeUnit.SECONDS, syncPeriod, TimeUnit.MILLISECONDS),
timeSource.convertDelay(TimeUnit.SECONDS, syncPeriod, TimeUnit.MILLISECONDS),
TimeUnit.MILLISECONDS);
@@ -463,11 +465,13 @@ public class SolrRrdBackendFactory extends RrdBackendFactory implements SolrClos
log.debug("Closing {}", hashCode());
}
closed = true;
+ syncService.shutdown();
+
+ scheduledFuture.cancel(true);
- syncService.shutdownNow();
try (ParWork closer = new ParWork(this)) {
closer.collect(backends.values());
- closer.collect(syncService);
+ closer.collect();
closer.collect(syncService);
}
backends.clear();
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
index bb0775a..f10f846 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -700,11 +700,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
@Override
public void close() {
- // even though we skip closes, we let local tests know not to close so that a full understanding can take
- // place
- assert Thread.currentThread().getStackTrace()[2].getClassName().matches(
- "org\\.apache\\.(?:solr|lucene).*") ? false : true : CLOSE_STREAM_MSG;
- this.stream = ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM;
+ // don't allow close
}
}
@@ -715,12 +711,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
@Override
public void close() {
- // even though we skip closes, we let local tests know not to close so that a full understanding can take
- // place
- assert Thread.currentThread().getStackTrace()[2].getClassName().matches(
- "org\\.apache\\.(?:solr|lucene).*") ? false
- : true : CLOSE_STREAM_MSG;
- stream = ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM;
+ // don't allow close
}
}
}
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
index 98fc2e5..b3d8119 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
@@ -530,11 +530,7 @@ public class SolrRequestParsers {
@Override
public InputStream getStream() throws IOException {
- // we explicitly protect this servlet stream from being closed
- // so that it does not trip our test assert in our close shield
- // in SolrDispatchFilter - we must allow closes from getStream
- // due to the other impls of ContentStream
- return new CloseShieldInputStream(req.getInputStream());
+ return req.getInputStream();
}
}
@@ -681,8 +677,7 @@ public class SolrRequestParsers {
final Charset charset = (cs == null) ? StandardCharsets.UTF_8 : Charset.forName(cs);
FastInputStream fin = null;
try {
- // Protect container owned streams from being closed by us, see SOLR-8933
- fin = FastInputStream.wrap( in == null ? new CloseShieldInputStream(req.getInputStream()) : in );
+ fin = FastInputStream.wrap( in == null ? req.getInputStream() : in );
final long bytesRead = parseFormDataContent(fin, maxLength, charset, map, false);
if (bytesRead == 0L && totalLength > 0L) {
diff --git a/solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java b/solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java
index c074da5..b9eb6b2 100644
--- a/solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java
+++ b/solr/core/src/java/org/apache/solr/util/SafeXMLParsing.java
@@ -60,7 +60,7 @@ public final class SafeXMLParsing {
/** Parses the given InputStream as XML, disabling any external entities with secure processing enabled.
* The given InputStream is not closed. */
public static Document parseUntrustedXML(Logger log, InputStream in) throws SAXException, IOException {
- return getUntrustedDocumentBuilder(log).parse(new CloseShieldInputStream(in), SYSTEMID_UNTRUSTED);
+ return getUntrustedDocumentBuilder(log).parse(in, SYSTEMID_UNTRUSTED);
}
/** Parses the given InputStream as XML, disabling any external entities with secure processing enabled.