shaofengshi closed pull request #274: KYLIN-3597 Improve code smell
URL: https://github.com/apache/kylin/pull/274
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/core-common/src/main/java/org/apache/kylin/common/persistence/IdentifierFileResourceStore.java
b/core-common/src/main/java/org/apache/kylin/common/persistence/IdentifierFileResourceStore.java
index e516bd1149..9475e44189 100644
---
a/core-common/src/main/java/org/apache/kylin/common/persistence/IdentifierFileResourceStore.java
+++
b/core-common/src/main/java/org/apache/kylin/common/persistence/IdentifierFileResourceStore.java
@@ -17,8 +17,6 @@
*/
package org.apache.kylin.common.persistence;
-import java.io.File;
-
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.StorageURL;
import org.slf4j.Logger;
@@ -38,8 +36,6 @@
private static final String IFILE_SCHEME = "ifile";
- private File root;
-
public IdentifierFileResourceStore(KylinConfig kylinConfig) throws
Exception {
super(kylinConfig);
}
diff --git
a/core-cube/src/main/java/org/apache/kylin/gridtable/GTFilterScanner.java
b/core-cube/src/main/java/org/apache/kylin/gridtable/GTFilterScanner.java
index 12074bd850..89d29e3627 100644
--- a/core-cube/src/main/java/org/apache/kylin/gridtable/GTFilterScanner.java
+++ b/core-cube/src/main/java/org/apache/kylin/gridtable/GTFilterScanner.java
@@ -72,75 +72,74 @@ public long getInputRowCount() {
@Override
public Iterator<GTRecord> iterator() {
- return new Iterator<GTRecord>() {
-
- private Iterator<GTRecord> inputIterator = delegated.iterator();
- private FilterResultCache resultCache = new
FilterResultCache(getInfo(), filter);
-
- @Override
- public boolean hasNext() {
- if (next != null)
- return true;
-
- while (inputIterator.hasNext()) {
- next = inputIterator.next();
- inputRowCount++;
- if (!evaluate()) {
- continue;
- }
- return true;
- }
- next = null;
- return false;
- }
+ return new GTFilterScannerIterator();
+ }
- private boolean evaluate() {
- if (checker != null && checker.shouldBypass(next)) {
- return false;
- }
+ private class GTFilterScannerIterator implements Iterator<GTRecord> {
+ private Iterator<GTRecord> inputIterator = delegated.iterator();
+ private FilterResultCache resultCache = new
FilterResultCache(getInfo(), filter);
- if (filter == null)
- return true;
+ @Override
+ public boolean hasNext() {
+ if (next != null)
+ return true;
- // 'next' and 'oneTuple' are referring to the same record
- boolean[] cachedResult = resultCache.checkCache(next);
- if (cachedResult != null)
- return cachedResult[0];
+ while (inputIterator.hasNext()) {
+ next = inputIterator.next();
+ inputRowCount++;
+ if (!evaluate()) {
+ continue;
+ }
+ return true;
+ }
+ next = null;
+ return false;
+ }
- boolean result = filter.evaluate(oneTuple, filterCodeSystem);
- resultCache.setLastResult(result);
- return result;
+ private boolean evaluate() {
+ if (checker != null && checker.shouldBypass(next)) {
+ return false;
}
- @Override
- public GTRecord next() {
- // fetch next record
- if (next == null) {
- hasNext();
- if (next == null)
- throw new NoSuchElementException();
- }
+ if (filter == null)
+ return true;
- GTRecord result = next;
- next = null;
- return result;
- }
+ // 'next' and 'oneTuple' are referring to the same record
+ boolean[] cachedResult = resultCache.checkCache(next);
+ if (cachedResult != null)
+ return cachedResult[0];
+
+ boolean result = filter.evaluate(oneTuple, filterCodeSystem);
+ resultCache.setLastResult(result);
+ return result;
+ }
- @Override
- public void remove() {
- throw new UnsupportedOperationException();
+ @Override
+ public GTRecord next() {
+ // fetch next record
+ if (next == null) {
+ hasNext();
+ if (next == null)
+ throw new NoSuchElementException();
}
- };
+ GTRecord result = next;
+ next = null;
+ return result;
+ }
+
+ @Override
+ public void remove() {
+ throw new UnsupportedOperationException();
+ }
}
// cache the last one input and result, can reuse because rowkey are
ordered, and same input could come in small group
public static class FilterResultCache {
static final int CHECKPOINT = 10000;
static final double HIT_RATE_THRESHOLD = 0.5;
- public static boolean ENABLED = true; // enable cache by default
-
- public boolean enabled = ENABLED;
+ public static boolean DEFAULT_OPTION = true; // enable cache by default
+ private boolean enabled = DEFAULT_OPTION;
ImmutableBitSet colsInFilter;
int count;
int hit;
diff --git a/core-cube/src/main/java/org/apache/kylin/gridtable/GTUtil.java
b/core-cube/src/main/java/org/apache/kylin/gridtable/GTUtil.java
index 62053a3007..f03329b2f8 100644
--- a/core-cube/src/main/java/org/apache/kylin/gridtable/GTUtil.java
+++ b/core-cube/src/main/java/org/apache/kylin/gridtable/GTUtil.java
@@ -46,6 +46,8 @@
public class GTUtil {
+ private GTUtil(){}
+
static final TableDesc MOCKUP_TABLE = TableDesc.mockup("GT_MOCKUP_TABLE");
static TblColRef tblColRef(int col, String datatype) {
@@ -185,14 +187,14 @@ public ByteArray deserialize(ByteBuffer buffer) {
protected final Set<TblColRef> unevaluatableColumnCollector;
protected final Map<TblColRef, Integer> colMapping;
protected final GTInfo info;
- protected final boolean encodeConstants;
+ protected final boolean useEncodeConstants;
public GTConvertDecorator(Set<TblColRef> unevaluatableColumnCollector,
Map<TblColRef, Integer> colMapping,
GTInfo info, boolean encodeConstants) {
this.unevaluatableColumnCollector = unevaluatableColumnCollector;
this.colMapping = colMapping;
this.info = info;
- this.encodeConstants = encodeConstants;
+ this.useEncodeConstants = encodeConstants;
buf = ByteBuffer.allocate(info.getMaxColumnLength());
}
@@ -228,7 +230,7 @@ public TupleFilter onSerialize(TupleFilter filter) {
}
// encode constants
- if (encodeConstants && filter instanceof CompareTupleFilter) {
+ if (useEncodeConstants && filter instanceof CompareTupleFilter) {
return encodeConstants((CompareTupleFilter) filter);
}
diff --git
a/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/SortedIteratorMergerWithLimit.java
b/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/SortedIteratorMergerWithLimit.java
index b54cb2f4af..9baee149c1 100644
---
a/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/SortedIteratorMergerWithLimit.java
+++
b/core-storage/src/main/java/org/apache/kylin/storage/gtrecord/SortedIteratorMergerWithLimit.java
@@ -44,7 +44,6 @@
*/
public class SortedIteratorMergerWithLimit<E extends Cloneable> extends
SortedIteratorMerger<E> {
private int limit;
- private Comparator<E> comparator;
public SortedIteratorMergerWithLimit(Iterator<Iterator<E>> shardSubsets,
int limit, Comparator<E> comparator) {
super(shardSubsets, comparator);
@@ -52,6 +51,7 @@ public SortedIteratorMergerWithLimit(Iterator<Iterator<E>>
shardSubsets, int lim
this.comparator = comparator;
}
+ @Override
public Iterator<E> getIterator() {
return new MergedIteratorWithLimit(limit, comparator);
}
diff --git
a/core-storage/src/test/java/org/apache/kylin/storage/gtrecord/DictGridTableTest.java
b/core-storage/src/test/java/org/apache/kylin/storage/gtrecord/DictGridTableTest.java
index 08bcb65600..14c5eefc76 100644
---
a/core-storage/src/test/java/org/apache/kylin/storage/gtrecord/DictGridTableTest.java
+++
b/core-storage/src/test/java/org/apache/kylin/storage/gtrecord/DictGridTableTest.java
@@ -369,13 +369,13 @@ public void testFilterScannerPerf() throws IOException {
CompareTupleFilter fComp2 = compare(info.colRef(1),
FilterOperatorEnum.GT, enc(info, 1, "10"));
LogicalTupleFilter filter = and(fComp1, fComp2);
- FilterResultCache.ENABLED = false;
+ FilterResultCache.DEFAULT_OPTION = false;
testFilterScannerPerfInner(table, info, filter);
- FilterResultCache.ENABLED = true;
+ FilterResultCache.DEFAULT_OPTION = true;
testFilterScannerPerfInner(table, info, filter);
- FilterResultCache.ENABLED = false;
+ FilterResultCache.DEFAULT_OPTION = false;
testFilterScannerPerfInner(table, info, filter);
- FilterResultCache.ENABLED = true;
+ FilterResultCache.DEFAULT_OPTION = true;
testFilterScannerPerfInner(table, info, filter);
}
@@ -393,7 +393,7 @@ private void testFilterScannerPerfInner(GridTable table,
GTInfo info, LogicalTup
scanner.close();
long end = System.currentTimeMillis();
System.out.println(
- (end - start) + "ms with filter cache enabled=" +
FilterResultCache.ENABLED + ", " + i + " rows");
+ (end - start) + "ms with filter cache enabled=" +
FilterResultCache.DEFAULT_OPTION + ", " + i + " rows");
}
@Test
diff --git
a/server-base/src/main/java/org/apache/kylin/rest/controller/CubeController.java
b/server-base/src/main/java/org/apache/kylin/rest/controller/CubeController.java
index a78f26a209..b76f7b9268 100644
---
a/server-base/src/main/java/org/apache/kylin/rest/controller/CubeController.java
+++
b/server-base/src/main/java/org/apache/kylin/rest/controller/CubeController.java
@@ -299,7 +299,7 @@ public CubeInstance rebuildLookupSnapshot(@PathVariable
String cubeName, @PathVa
@RequestMapping(value = "/{cubeName}/refresh_lookup", method = {
RequestMethod.PUT }, produces = {
"application/json" })
@ResponseBody
- public JobInstance reBuildLookupSnapshot(@PathVariable String cubeName,
+ public JobInstance rebuildLookupSnapshot(@PathVariable String cubeName,
@RequestBody LookupSnapshotBuildRequest request) {
try {
final CubeManager cubeMgr = cubeService.getCubeManager();
@@ -365,16 +365,12 @@ public JobInstance rebuild(@PathVariable String cubeName,
@RequestBody JobBuildR
@RequestMapping(value = "/{cubeName}/build2", method = { RequestMethod.PUT
}, produces = { "application/json" })
@ResponseBody
public JobInstance build2(@PathVariable String cubeName, @RequestBody
JobBuildRequest2 req) {
- boolean existKafkaClient = false;
try {
Class<?> clazz =
Class.forName("org.apache.kafka.clients.consumer.KafkaConsumer");
- if (clazz != null) {
- existKafkaClient = true;
+ if (clazz == null) {
+ throw new ClassNotFoundException();
}
} catch (ClassNotFoundException e) {
- existKafkaClient = false;
- }
- if (!existKafkaClient) {
throw new InternalErrorException("Could not find Kafka
dependency");
}
return rebuild2(cubeName, req);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
