From a9475063a19f8a16f87b01c05c9daa08af444a0b Mon Sep 17 00:00:00 2001 From: Pushkar Raste Date: Tue, 31 Jan 2017 21:01:20 +0000 Subject: [PATCH] Findbugs analysis --- .../solr/analytics/util/MedianCalculator.java | 4 ++- .../util/valuesource/ConstDoubleSource.java | 3 +- .../dataimport/TemplateTransformer.java | 1 - .../TestFileListEntityProcessor.java | 5 ++-- .../extraction/XLSXResponseWriter.java | 2 +- .../apache/solr/hadoop/SolrOutputFormat.java | 13 ++++---- .../solr/cloud/CreateCollectionCmd.java | 4 ++- .../org/apache/solr/cloud/ZkController.java | 30 +++++++++---------- .../org/apache/solr/core/BlobRepository.java | 5 ++-- .../org/apache/solr/core/ConfigOverlay.java | 3 +- .../handler/component/FieldFacetStats.java | 8 ++--- .../solr/internal/csv/writer/CSVConfig.java | 2 +- .../org/apache/solr/request/SimpleFacets.java | 2 +- .../org/apache/solr/search/CacheConfig.java | 6 ++-- .../apache/solr/search/facet/LegacyFacet.java | 2 +- .../solr/search/function/OrdFieldSource.java | 2 +- .../org/apache/solr/update/CdcrUpdateLog.java | 4 +-- .../java/org/apache/solr/update/PeerSync.java | 6 ++-- .../apache/solr/util/CommandOperation.java | 2 +- .../apache/solr/util/ConcurrentLFUCache.java | 3 +- .../apache/solr/util/ConcurrentLRUCache.java | 3 +- .../solr/cloud/TestHashPartitioner.java | 5 ++-- .../TestLeaderInitiatedRecoveryThread.java | 1 + .../solr/cloud/TestRandomFlRTGCloud.java | 2 +- .../solr/cloud/hdfs/HdfsRecoverLeaseTest.java | 1 + .../apache/solr/core/TestConfigOverlay.java | 5 ++-- .../DistributedFacetPivotSmallTest.java | 2 +- .../org/apache/solr/search/TestDocSet.java | 3 +- .../solr/search/TestStressRecovery.java | 3 +- .../ParsingFieldUpdateProcessorsTest.java | 12 ++++---- .../util/hll/IntegrationTestGenerator.java | 1 + .../solrj/beans/DocumentObjectBinder.java | 5 ++-- .../solrj/io/stream/TextLogitStream.java | 2 +- .../client/solrj/request/UpdateRequest.java | 2 +- .../solr/common/params/MapSolrParams.java | 4 ++- .../solr/common/util/TestNamedListCodec.java | 1 - 36 files changed, 87 insertions(+), 72 deletions(-) diff --git a/solr/contrib/analytics/src/java/org/apache/solr/analytics/util/MedianCalculator.java b/solr/contrib/analytics/src/java/org/apache/solr/analytics/util/MedianCalculator.java index 52935e90dc00..e1080b9cff1b 100644 --- a/solr/contrib/analytics/src/java/org/apache/solr/analytics/util/MedianCalculator.java +++ b/solr/contrib/analytics/src/java/org/apache/solr/analytics/util/MedianCalculator.java @@ -35,7 +35,9 @@ public static > double getMedian(List list) select(list, .5 * size, 0, size); int firstIdx = (int) (Math.floor(.5 * size)); - int secondIdx = (firstIdx <= size && size % 2 == 1) ? firstIdx + 1 : firstIdx; + // from findbug bug info: + // The code uses x % 2 == 1 to check to see if a value is odd, but this won't work for negative numbers (e.g., (-5) % 2 == -1). If this code is intending to check for oddness, consider using x & 1 == 1, or x % 2 != 0. + int secondIdx = (firstIdx <= size && size % 2 != 0) ? firstIdx + 1 : firstIdx; double result = list.get(firstIdx).doubleValue() * .5 + list.get(secondIdx).doubleValue() * .5; return result; diff --git a/solr/contrib/analytics/src/java/org/apache/solr/analytics/util/valuesource/ConstDoubleSource.java b/solr/contrib/analytics/src/java/org/apache/solr/analytics/util/valuesource/ConstDoubleSource.java index 80e8ed183cfd..e0ebad64a11b 100644 --- a/solr/contrib/analytics/src/java/org/apache/solr/analytics/util/valuesource/ConstDoubleSource.java +++ b/solr/contrib/analytics/src/java/org/apache/solr/analytics/util/valuesource/ConstDoubleSource.java @@ -23,7 +23,6 @@ import org.apache.lucene.queries.function.FunctionValues; import org.apache.lucene.queries.function.docvalues.DoubleDocValues; import org.apache.lucene.queries.function.valuesource.ConstNumberSource; -import org.apache.lucene.queries.function.valuesource.ConstValueSource; import org.apache.solr.analytics.util.AnalyticsParams; /** @@ -67,7 +66,7 @@ public int hashCode() { @Override public boolean equals(Object o) { - if (!(o instanceof ConstValueSource)) return false; + if (!(o instanceof ConstDoubleSource)) return false; ConstDoubleSource other = (ConstDoubleSource)o; return this.constant == other.constant; } diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/TemplateTransformer.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/TemplateTransformer.java index a5faa7e97b4f..fb3df80cb464 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/TemplateTransformer.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/TemplateTransformer.java @@ -60,7 +60,6 @@ public Object transformRow(Map row, Context context) { // Add current row to the copy of resolver map for (Map map : context.getAllEntityFields()) { - map.entrySet(); String expr = map.get(TEMPLATE); if (expr == null) continue; diff --git a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestFileListEntityProcessor.java b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestFileListEntityProcessor.java index dd2cf72f6421..bf1ea4cf6a4f 100644 --- a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestFileListEntityProcessor.java +++ b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestFileListEntityProcessor.java @@ -70,8 +70,9 @@ public void testSimple() throws IOException { @Test public void testBiggerSmallerFiles() throws IOException { File tmpdir = createTempDir().toFile(); - - long minLength = Long.MAX_VALUE; + // array length can never be great than Integer.MAX_VALUE, + // was using Long.MAX_VALUE intentional + int minLength = Integer.MAX_VALUE; String smallestFile = ""; byte[] content = "abcdefgij".getBytes(StandardCharsets.UTF_8); createFile(tmpdir, "a.xml", content, false); diff --git a/solr/contrib/extraction/src/java/org/apache/solr/handler/extraction/XLSXResponseWriter.java b/solr/contrib/extraction/src/java/org/apache/solr/handler/extraction/XLSXResponseWriter.java index 27a30d170e06..2e544e08b446 100644 --- a/solr/contrib/extraction/src/java/org/apache/solr/handler/extraction/XLSXResponseWriter.java +++ b/solr/contrib/extraction/src/java/org/apache/solr/handler/extraction/XLSXResponseWriter.java @@ -357,7 +357,7 @@ public void writeArray(String name, Iterator val) throws IOException { if (v instanceof IndexableField) { IndexableField f = (IndexableField)v; if (v instanceof Date) { - output.append(((Date) val).toInstant().toString() + "; "); + output.append(((Date) v).toInstant().toString() + "; "); } else { output.append(f.stringValue() + "; "); } diff --git a/solr/contrib/map-reduce/src/java/org/apache/solr/hadoop/SolrOutputFormat.java b/solr/contrib/map-reduce/src/java/org/apache/solr/hadoop/SolrOutputFormat.java index b52939e3f9d0..ad06c8dca277 100644 --- a/solr/contrib/map-reduce/src/java/org/apache/solr/hadoop/SolrOutputFormat.java +++ b/solr/contrib/map-reduce/src/java/org/apache/solr/hadoop/SolrOutputFormat.java @@ -257,12 +257,13 @@ private static void listFiles(File dir, Set files) throws IOException { files.add(dir); return; } - - for (File f : list) { - if (f.isFile()) { - files.add(f); - } else { - listFiles(f, files); + if(list != null) { + for (File f : list) { + if (f.isFile()) { + files.add(f); + } else { + listFiles(f, files); + } } } } diff --git a/solr/core/src/java/org/apache/solr/cloud/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/CreateCollectionCmd.java index a1bb70e36ab9..e7b8ada62211 100644 --- a/solr/core/src/java/org/apache/solr/cloud/CreateCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/CreateCollectionCmd.java @@ -296,7 +296,9 @@ String getConfigName(String coll, ZkNodeProps message) throws KeeperException, I configName = configNames.get(0); // no config set named, but there is only 1 - use it log.info("Only one config set found in zk - using it:" + configName); - } else if (configNames.contains(coll)) { + } + // make sure that we haven't reached here due to (configNames != null) check failed (i.e. configNames is null) + else if (configNames != null && configNames.contains(coll)) { configName = coll; } } catch (KeeperException.NoNodeException e) { diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index eba7067a90a0..4192f9aaacfd 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -1197,12 +1197,10 @@ public void publish(final CoreDescriptor cd, final Replica.State state, boolean } try (SolrCore core = cc.getCore(cd.getName())) { if (core != null && core.getDirectoryFactory().isSharedStorage()) { - if (core != null && core.getDirectoryFactory().isSharedStorage()) { - props.put("dataDir", core.getDataDir()); - UpdateLog ulog = core.getUpdateHandler().getUpdateLog(); - if (ulog != null) { - props.put("ulogDir", ulog.getLogDir()); - } + props.put("dataDir", core.getDataDir()); + UpdateLog ulog = core.getUpdateHandler().getUpdateLog(); + if (ulog != null) { + props.put("ulogDir", ulog.getLogDir()); } } } @@ -1753,16 +1751,18 @@ public void rejoinShardLeaderElection(SolrParams params) { if (prevContext != null) prevContext.cancelElection(); ZkNodeProps zkProps = new ZkNodeProps(BASE_URL_PROP, baseUrl, CORE_NAME_PROP, coreName, NODE_NAME_PROP, getNodeName(), CORE_NODE_NAME_PROP, coreNodeName); - - LeaderElector elect = ((ShardLeaderElectionContextBase) prevContext).getLeaderElector(); - ShardLeaderElectionContext context = new ShardLeaderElectionContext(elect, shardId, collectionName, - coreNodeName, zkProps, this, getCoreContainer()); - - context.leaderSeqPath = context.electionPath + LeaderElector.ELECTION_NODE + "/" + electionNode; - elect.setup(context); - electionContexts.put(contextKey, context); - elect.retryElection(context, params.getBool(REJOIN_AT_HEAD_PROP, false)); + if(prevContext != null) { + LeaderElector elect = ((ShardLeaderElectionContextBase) prevContext).getLeaderElector(); + ShardLeaderElectionContext context = new ShardLeaderElectionContext(elect, shardId, collectionName, + coreNodeName, zkProps, this, getCoreContainer()); + + context.leaderSeqPath = context.electionPath + LeaderElector.ELECTION_NODE + "/" + electionNode; + elect.setup(context); + electionContexts.put(contextKey, context); + + elect.retryElection(context, params.getBool(REJOIN_AT_HEAD_PROP, false)); + } } } catch (Exception e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Unable to rejoin election", e); diff --git a/solr/core/src/java/org/apache/solr/core/BlobRepository.java b/solr/core/src/java/org/apache/solr/core/BlobRepository.java index bbe7c628f53e..ab114eb76e75 100644 --- a/solr/core/src/java/org/apache/solr/core/BlobRepository.java +++ b/solr/core/src/java/org/apache/solr/core/BlobRepository.java @@ -110,7 +110,8 @@ BlobContentRef getBlobIncRef(String key, Decoder decoder) { private BlobContentRef getBlobIncRef(String key, Callable> blobCreator) { BlobContent aBlob; if (this.coreContainer.isZooKeeperAware()) { - synchronized (blobs) { + //do we really need lock on a concurrent map? + //synchronized (blobs) { aBlob = blobs.get(key); if (aBlob == null) { try { @@ -119,7 +120,7 @@ private BlobContentRef getBlobIncRef(String key, Callable> throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Blob loading failed: "+e.getMessage(), e); } } - } + //} } else { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Blob loading is not supported in non-cloud mode"); // todo diff --git a/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java b/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java index 34729995c638..342d243627b1 100644 --- a/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java +++ b/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java @@ -215,7 +215,8 @@ private static Class checkType(Object o, boolean isXpath, boolean isAttr) { } } - public Map getEditableSubProperties(String xpath) { + // Not all properties are string, some are Integer and some Boolean etc + public Map getEditableSubProperties(String xpath) { Object o = Utils.getObjectByPath(props, false, StrUtils.splitSmart(xpath, '/')); if (o instanceof Map) { return (Map) o; diff --git a/solr/core/src/java/org/apache/solr/handler/component/FieldFacetStats.java b/solr/core/src/java/org/apache/solr/handler/component/FieldFacetStats.java index 2b8373a20204..b1333b7a448f 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/FieldFacetStats.java +++ b/solr/core/src/java/org/apache/solr/handler/component/FieldFacetStats.java @@ -112,11 +112,9 @@ public boolean facetTermNum(int docID, int statsTermNum) throws IOException { int arrIdx = term; if (arrIdx >= 0 && arrIdx < topLevelSortedValues.getValueCount()) { final String key; - if (term == -1) { - key = null; - } else { - key = topLevelSortedValues.lookupOrd(term).utf8ToString(); - } + // if arrIdx > 0 and with had we had assigned term to arrIdx (on line 112 -> arrIdx = term) + // aren't we sure that term > -1 + key = topLevelSortedValues.lookupOrd(term).utf8ToString(); while (facetStatsTerms.size() <= statsTermNum) { facetStatsTerms.add(new HashMap()); } diff --git a/solr/core/src/java/org/apache/solr/internal/csv/writer/CSVConfig.java b/solr/core/src/java/org/apache/solr/internal/csv/writer/CSVConfig.java index 354bd3c9f3b5..92a7a375ebfc 100644 --- a/solr/core/src/java/org/apache/solr/internal/csv/writer/CSVConfig.java +++ b/solr/core/src/java/org/apache/solr/internal/csv/writer/CSVConfig.java @@ -246,7 +246,7 @@ public void setFieldHeader(boolean fieldHeader) { */ @Override public boolean equals(Object obj) { - if (obj == null && !(obj instanceof CSVConfig)) { + if (!(obj instanceof CSVConfig)) { return false; } return super.equals(obj); diff --git a/solr/core/src/java/org/apache/solr/request/SimpleFacets.java b/solr/core/src/java/org/apache/solr/request/SimpleFacets.java index 641b1f394020..15d06221a675 100644 --- a/solr/core/src/java/org/apache/solr/request/SimpleFacets.java +++ b/solr/core/src/java/org/apache/solr/request/SimpleFacets.java @@ -207,7 +207,7 @@ protected ParsedParams parseParams(String type, String param) throws SyntaxError SolrParams required = new RequiredSolrParams(params); // remove local params unless it's a query - if (type != FacetParams.FACET_QUERY) { // TODO Cut over to an Enum here + if (!FacetParams.FACET_QUERY.equals(type)) { // TODO Cut over to an Enum here facetValue = localParams.get(CommonParams.VALUE); } diff --git a/solr/core/src/java/org/apache/solr/search/CacheConfig.java b/solr/core/src/java/org/apache/solr/search/CacheConfig.java index d3565a691ae3..1cbd7810ad17 100644 --- a/solr/core/src/java/org/apache/solr/search/CacheConfig.java +++ b/solr/core/src/java/org/apache/solr/search/CacheConfig.java @@ -90,7 +90,7 @@ public static Map getMultipleConfigs(SolrConfig solrConfig, public static CacheConfig getConfig(SolrConfig solrConfig, String xpath) { Node node = solrConfig.getNode(xpath, false); if(node == null) { - Map m = solrConfig.getOverlay().getEditableSubProperties(xpath); + Map m = solrConfig.getOverlay().getEditableSubProperties(xpath); if(m==null) return null; List parts = StrUtils.splitSmart(xpath, '/'); return getConfig(solrConfig,parts.get(parts.size()-1) , Collections.EMPTY_MAP,xpath); @@ -109,10 +109,10 @@ public static CacheConfig getConfig(SolrConfig solrConfig, String nodeName, Map< attrs = attrsCopy; config.args = attrs; - Map map = xpath == null ? null : solrConfig.getOverlay().getEditableSubProperties(xpath); + Map map = xpath == null ? null : solrConfig.getOverlay().getEditableSubProperties(xpath); if(map != null){ HashMap mapCopy = new HashMap<>(config.args); - for (Map.Entry e : map.entrySet()) { + for (Map.Entry e : map.entrySet()) { mapCopy.put(e.getKey(),String.valueOf(e.getValue())); } config.args = mapCopy; diff --git a/solr/core/src/java/org/apache/solr/search/facet/LegacyFacet.java b/solr/core/src/java/org/apache/solr/search/facet/LegacyFacet.java index 9457d9c4cc02..3a3a9b4cb3c1 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/LegacyFacet.java +++ b/solr/core/src/java/org/apache/solr/search/facet/LegacyFacet.java @@ -296,7 +296,7 @@ protected void parseParams(String type, String param) { required = new RequiredSolrParams(params); // remove local params unless it's a query - if (type != FacetParams.FACET_QUERY) { + if (!FacetParams.FACET_QUERY.equals(type)) { facetValue = localParams.get(CommonParams.VALUE); } diff --git a/solr/core/src/java/org/apache/solr/search/function/OrdFieldSource.java b/solr/core/src/java/org/apache/solr/search/function/OrdFieldSource.java index 756a1a690586..f6431c53cc21 100644 --- a/solr/core/src/java/org/apache/solr/search/function/OrdFieldSource.java +++ b/solr/core/src/java/org/apache/solr/search/function/OrdFieldSource.java @@ -113,7 +113,7 @@ private int getOrdForDoc(int docID) throws IOException { return -1; } } - + protected String toTerm(String readableValue) { return readableValue; } diff --git a/solr/core/src/java/org/apache/solr/update/CdcrUpdateLog.java b/solr/core/src/java/org/apache/solr/update/CdcrUpdateLog.java index 6b202044d760..a39920cf61ba 100644 --- a/solr/core/src/java/org/apache/solr/update/CdcrUpdateLog.java +++ b/solr/core/src/java/org/apache/solr/update/CdcrUpdateLog.java @@ -758,8 +758,8 @@ public void resetToLastPosition() { */ public long getNumberOfRemainingRecords() { long numRemainingRecords = 0; - - synchronized (tlogs) { + // do we really need lock on a LinkedBlockingQueue ? + synchronized (tlogs) { for (TransactionLog tlog : tlogs) { numRemainingRecords += tlog.numRecords() - 1; // minus 1 as the number of records returned by the tlog includes the header } diff --git a/solr/core/src/java/org/apache/solr/update/PeerSync.java b/solr/core/src/java/org/apache/solr/update/PeerSync.java index 861cbf754549..358f941a1c78 100644 --- a/solr/core/src/java/org/apache/solr/update/PeerSync.java +++ b/solr/core/src/java/org/apache/solr/update/PeerSync.java @@ -840,12 +840,12 @@ private boolean handleUpdates(ShardResponse srsp) { // TODO: should this be handled separately as a problem with us? // I guess it probably already will by causing replication to be kicked off. sreq.updateException = e; - log.error(msg() + "Error applying updates from " + sreq.shards + " ,update=" + o, e); + log.error(msg() + "Error applying updates from " + sreq.shards[0] + " ,update=" + o, e); return false; } catch (Exception e) { sreq.updateException = e; - log.error(msg() + "Error applying updates from " + sreq.shards + " ,update=" + o, e); + log.error(msg() + "Error applying updates from " + sreq.shards[0] + " ,update=" + o, e); return false; } finally { @@ -853,7 +853,7 @@ private boolean handleUpdates(ShardResponse srsp) { proc.finish(); } catch (Exception e) { sreq.updateException = e; - log.error(msg() + "Error applying updates from " + sreq.shards + " ,finish()", e); + log.error(msg() + "Error applying updates from " + sreq.shards[0] + " ,finish()", e); return false; } } diff --git a/solr/core/src/java/org/apache/solr/util/CommandOperation.java b/solr/core/src/java/org/apache/solr/util/CommandOperation.java index 6b8f14f3da80..d6e1974628e9 100644 --- a/solr/core/src/java/org/apache/solr/util/CommandOperation.java +++ b/solr/core/src/java/org/apache/solr/util/CommandOperation.java @@ -53,7 +53,7 @@ public CommandOperation(String operationName, Object metaData) { public String getStr(String key, String def) { if (ROOT_OBJ.equals(key)) { Object obj = getRootPrimitive(); - return obj == def ? null : String.valueOf(obj); + return obj == def ? null : String.valueOf(obj); } Object o = getMapVal(key); return o == null ? def : String.valueOf(o); diff --git a/solr/core/src/java/org/apache/solr/util/ConcurrentLFUCache.java b/solr/core/src/java/org/apache/solr/util/ConcurrentLFUCache.java index 6f2ff2d09bab..33ce89f68fa0 100644 --- a/solr/core/src/java/org/apache/solr/util/ConcurrentLFUCache.java +++ b/solr/core/src/java/org/apache/solr/util/ConcurrentLFUCache.java @@ -157,7 +157,8 @@ private void markAndSweep() { try { long lowHitCount = this.lowHitCount; isCleaning = true; - this.lowHitCount = lowHitCount; // volatile write to make isCleaning visible + // (can't we just make isCleaning volatile?, besides lowHitCount is not volatile at all) + this.lowHitCount = lowHitCount; // volatile write to make isCleaning visible int sz = stats.size.get(); if (sz <= upperWaterMark) { diff --git a/solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java b/solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java index e8758287910a..716a047db710 100644 --- a/solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java +++ b/solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java @@ -262,7 +262,8 @@ private void markAndSweepByRamSize() { private void markAndSweepByCacheSize() { long oldestEntry = this.oldestEntry; isCleaning = true; - this.oldestEntry = oldestEntry; // volatile write to make isCleaning visible + // (can't we just make isCleaning volatile?, besides lowHitCount is not volatile at all) + this.oldestEntry = oldestEntry; // volatile write to make isCleaning visible, long timeCurrent = stats.accessCounter.longValue(); int sz = stats.size.get(); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java b/solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java index c84c20488572..5d24015e4b12 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java @@ -57,9 +57,10 @@ public void testMapHashes() throws Exception { for (int i = 1; i <= 30000; i++) { // start skipping at higher numbers - if (i > 100) i+=13; + // if we are doing > comparison, shouldn't we compare starting form 5000 down to 100? + if (i > 5000) i+=101; else if (i > 1000) i+=31; - else if (i > 5000) i+=101; + else if (i > 100) i+=13; long rangeSize = 0x0000000100000000L / i; diff --git a/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java b/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java index 11858f828b7f..aed991a5cf5c 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java @@ -83,6 +83,7 @@ public boolean isLeader() { LeaderInitiatedRecoveryThread thread = new LeaderInitiatedRecoveryThread(zkController, coreContainer, DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, cd); assertFalse(zkController.isReplicaInRecoveryHandling(replicaCoreNodeProps.getCoreUrl())); + // did we mean thread.start() ??? thread.run(); fail("publishDownState should not have succeeded because replica url is not marked in leader initiated recovery in ZkController"); } catch (SolrException e) { diff --git a/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java b/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java index 966d8ef75c3f..5e2cc4bb1e0d 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestRandomFlRTGCloud.java @@ -298,7 +298,7 @@ private void assertDelete(final SolrInputDocument[] knownDocs, final int[] docId ids.add("" + docId); knownDocs[docId] = null; } - assertEquals("Failed delete: " + docIds, 0, getRandClient(random()).deleteById(ids).getStatus()); + assertEquals("Failed delete: " + Arrays.toString(docIds), 0, getRandClient(random()).deleteById(ids).getStatus()); } /** diff --git a/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsRecoverLeaseTest.java b/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsRecoverLeaseTest.java index fcacb8b5d587..acf13d0151bd 100644 --- a/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsRecoverLeaseTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsRecoverLeaseTest.java @@ -217,6 +217,7 @@ public void close() throws IOException { for (int i = 0; i < threadCount; i++) { WriterThread wt = new WriterThread(i); writerThreads.add(wt); + // did we mean wt.start() ??? wt.run(); } diff --git a/solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java b/solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java index 633dde3552d6..600ce5715420 100644 --- a/solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java +++ b/solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java @@ -67,10 +67,11 @@ public void testSetProperty(){ ConfigOverlay overlay = new ConfigOverlay(Collections.EMPTY_MAP,0); overlay = overlay.setProperty("query.filterCache.initialSize",100); assertEquals(100, overlay.getXPathProperty("query/filterCache/@initialSize")); - Map map = overlay.getEditableSubProperties("query/filterCache"); + Map map = overlay.getEditableSubProperties("query/filterCache"); assertNotNull(map); assertEquals(1,map.size()); - assertEquals(100,map.get("initialSize")); + assertEquals(Integer.valueOf(100), (Integer)map.get("initialSize")); + } diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotSmallTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotSmallTest.java index fc7af80dd60a..2e82b5b86813 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotSmallTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotSmallTest.java @@ -1596,7 +1596,7 @@ public boolean equals(Object obj) { if (getFacetQuery().size() != other.getFacetQuery().size()) return false; for (Map.Entry entry : getFacetQuery().entrySet()) { Integer otherQCount = other.getFacetQuery().get(entry.getKey()); - if (otherQCount == null || otherQCount != entry.getValue()) return false; + if (otherQCount == null || !otherQCount.equals(entry.getValue())) return false; } } return true; diff --git a/solr/core/src/test/org/apache/solr/search/TestDocSet.java b/solr/core/src/test/org/apache/solr/search/TestDocSet.java index 2849f09a2ce6..0cac07184f3a 100644 --- a/solr/core/src/test/org/apache/solr/search/TestDocSet.java +++ b/solr/core/src/test/org/apache/solr/search/TestDocSet.java @@ -59,7 +59,8 @@ public void setUp() throws Exception { // test the DocSetCollector public void collect(DocSet set, int maxDoc) { - int smallSetSize = maxDoc >> 64 + 3; + // why are we shifting 32 bit integer 67 times ? + int smallSetSize = maxDoc >> 64 + 3; if (set.size() > 1) { if (random().nextBoolean()) { smallSetSize = set.size() + random().nextInt(3) - 1; // test the bounds around smallSetSize diff --git a/solr/core/src/test/org/apache/solr/search/TestStressRecovery.java b/solr/core/src/test/org/apache/solr/search/TestStressRecovery.java index b6ecc2e75e25..760afcf8b0ea 100644 --- a/solr/core/src/test/org/apache/solr/search/TestStressRecovery.java +++ b/solr/core/src/test/org/apache/solr/search/TestStressRecovery.java @@ -272,7 +272,8 @@ public void run() { try { while (operations.get() > 0) { // throttle reads (don't completely stop) - readPermission.tryAcquire(10, TimeUnit.MILLISECONDS); + // should not we make sure that we acquired a permit ? + readPermission.tryAcquire(10, TimeUnit.MILLISECONDS); // bias toward a recently changed doc diff --git a/solr/core/src/test/org/apache/solr/update/processor/ParsingFieldUpdateProcessorsTest.java b/solr/core/src/test/org/apache/solr/update/processor/ParsingFieldUpdateProcessorsTest.java index b779f7a12d94..ceb9a87e9278 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/ParsingFieldUpdateProcessorsTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/ParsingFieldUpdateProcessorsTest.java @@ -251,7 +251,7 @@ public void testFailedParseMixedDate() throws Exception { assertNotNull(d); boolean foundDouble = false; for (Object o : d.getFieldValues("not_in_schema")) { - if (extraDouble == o) { + if (extraDouble.equals(o)) { foundDouble = true; } else { assertTrue(o instanceof String); @@ -349,7 +349,7 @@ public void testFailedParseMixedInt() throws Exception { assertNotNull(d); boolean foundFloat = false; for (Object o : d.getFieldValues("not_in_schema")) { - if (floatVal == o) { + if (floatVal.equals(o)) { foundFloat = true; } else { assertTrue(o instanceof String); @@ -432,7 +432,7 @@ public void testFailedParseMixedLong() throws Exception { assertNotNull(d); boolean foundFloat = false; for (Object o : d.getFieldValues("not_in_schema")) { - if (floatVal == o) { + if (floatVal.equals(o)) { foundFloat = true; } else { assertTrue(o instanceof String); @@ -534,7 +534,7 @@ public void testFailedParseMixedFloat() throws Exception { assertNotNull(d); boolean foundLong = false; for (Object o : d.getFieldValues("not_in_schema")) { - if (longVal == o) { + if (longVal.equals(o)) { foundLong = true; } else { assertTrue(o instanceof String); @@ -618,7 +618,7 @@ public void testFailedParseMixedDouble() throws Exception { assertNotNull(d); boolean foundLong = false; for (Object o : d.getFieldValues("not_in_schema")) { - if (longVal == o) { + if (longVal.equals(o)) { foundLong = true; } else { assertTrue(o instanceof String); @@ -720,7 +720,7 @@ public void testFailedParseMixedBoolean() throws Exception { assertNotNull(d); boolean foundLong = false; for (Object o : d.getFieldValues("not_in_schema")) { - if (longVal == o) { + if (longVal.equals(o)) { foundLong = true; } else { assertTrue(o instanceof String); diff --git a/solr/core/src/test/org/apache/solr/util/hll/IntegrationTestGenerator.java b/solr/core/src/test/org/apache/solr/util/hll/IntegrationTestGenerator.java index 69757ce9312f..3917cc1d6209 100644 --- a/solr/core/src/test/org/apache/solr/util/hll/IntegrationTestGenerator.java +++ b/solr/core/src/test/org/apache/solr/util/hll/IntegrationTestGenerator.java @@ -503,6 +503,7 @@ public static void main(String[] args) throws IOException { * ({@link #LOG2M}, {@link #REGWIDTH}, {@link #EXPLICIT_THRESHOLD}, * and {@link #SPARSE_THRESHOLD}) specified above. */ + // aren't we in an infinite loop here? private static HLL newHLL(final HLLType type) { return newHLL(type); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java b/solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java index 9550a415776e..c63dff0e40f6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/beans/DocumentObjectBinder.java @@ -119,9 +119,10 @@ private void addChild(Object obj, DocField field, SolrInputDocument doc) { private List getDocFields(Class clazz) { List fields = infocache.get(clazz); if (fields == null) { - synchronized(infocache) { + //do we really need lock on a concurrent hashmap + //synchronized(infocache) { infocache.put(clazz, fields = collectInfo(clazz)); - } + //} } return fields; } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TextLogitStream.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TextLogitStream.java index c40f785ab612..51a1b8c97576 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TextLogitStream.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TextLogitStream.java @@ -103,7 +103,7 @@ public TextLogitStream(String zkHost, double threshold, int maxIterations) throws IOException { - init(collectionName, zkHost, params, name, field, termsStream, weights, outcome, positiveLabel, threshold, maxIterations, iteration); + init(collectionName, zkHost, params, name, field, termsStream, weights, outcome, positiveLabel, threshold, maxIterations, 0); } /** diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java index e7ca0fa32a0f..406efb5cebe9 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/UpdateRequest.java @@ -382,7 +382,7 @@ private List>> getDocLists(Map(); docLists.add(docList); diff --git a/solr/solrj/src/java/org/apache/solr/common/params/MapSolrParams.java b/solr/solrj/src/java/org/apache/solr/common/params/MapSolrParams.java index 5454fcac277b..6c4a44589f25 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/MapSolrParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/MapSolrParams.java @@ -45,7 +45,9 @@ public String get(String name) { @Override public String[] getParams(String name) { Object val = map.get(name); - if (val instanceof String[]) return (String[]) val; + // if map is defined as Map why would value be ever of type String[], + // should we fix map definition to be Map ? + if (val instanceof String[]) return (String[]) val; return val==null ? null : new String[]{String.valueOf(val)}; } diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestNamedListCodec.java b/solr/solrj/src/test/org/apache/solr/common/util/TestNamedListCodec.java index ae2a03645db7..1dbfe204ccb0 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/TestNamedListCodec.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/TestNamedListCodec.java @@ -96,7 +96,6 @@ public void testIterator() throws Exception{ String sval = "12qwaszx"; // Set up a simple document - NamedList r = new NamedList(); List list = new ArrayList(); SolrDocument doc = new SolrDocument();