----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31938/#review76138 -----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/ParallelizationInfo.java <https://reviews.apache.org/r/31938/#comment123594> You shouldn't need the this. here. exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java <https://reviews.apache.org/r/31938/#comment123595> Excellent. Thank you for improving that error message. exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/StatsCollector.java <https://reviews.apache.org/r/31938/#comment123597> add a final Stats stats = wrapper.getStats(); then reuse it stats.addMaxWidth(...); stats.addMinWidth(...); exec/java-exec/src/main/java/org/apache/drill/exec/store/RecordDataType.java <https://reviews.apache.org/r/31938/#comment123598> Can these locals be final? exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoDataType.java <https://reviews.apache.org/r/31938/#comment123599> Good code improvement. exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/StaticDrillTable.java <https://reviews.apache.org/r/31938/#comment123600> I like private loggers. exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemRecords.java <https://reviews.apache.org/r/31938/#comment123607> This, and all the nested static classes should be separate classes in this package. Each of them can have a private constructor, with a public final INSTANCE variable, similar to what you've already done. exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemRecords.java <https://reviews.apache.org/r/31938/#comment123602> final exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemRecords.java <https://reviews.apache.org/r/31938/#comment123604> final exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemRecords.java <https://reviews.apache.org/r/31938/#comment123605> final exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemRecords.java <https://reviews.apache.org/r/31938/#comment123606> get the endpoint once, and reuse it. then, get the mutators once each, and reuse them. The locals should be final. exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java <https://reviews.apache.org/r/31938/#comment123608> can this be final? exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java <https://reviews.apache.org/r/31938/#comment123611> Could this be a method on the SystemTable enum instead? exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java <https://reviews.apache.org/r/31938/#comment123610> can these locals be final? Also, can we keep them private to this case by using braces for the case block? exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java <https://reviews.apache.org/r/31938/#comment123612> Could this be a method on the SystemTable enum instead? exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTablePluginConfig.java <https://reviews.apache.org/r/31938/#comment123615> Good catch that this was missing for the singleton. exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableScan.java <https://reviews.apache.org/r/31938/#comment123616> final Does the sys.drillbits table list the host and port separately? If so, these should do so as well. If I'm misremembering that, ignore this. - Chris Westin On March 11, 2015, 10:04 a.m., Sudheesh Katkam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31938/ > ----------------------------------------------------------- > > (Updated March 11, 2015, 10:04 a.m.) > > > Review request for drill and Venki Korukanti. > > > Repository: drill-git > > > Description > ------- > > DRILL-2275: Added support to allow for querying cluster state information. > + If table isDistributed(), BatchCreator and SystemTableScan allow for a > distributed query. > + SystemRecordReader reads SystemRecords. > + There is now a generic data type for static tables. > + GroupScan can enforce width to be maximum width on ExcessiveExchangeRemover. > + GroupScan has minimum width for SimpleParallelizer. > > > Diffs > ----- > > common/src/main/java/org/apache/drill/common/JSONOptions.java 64e6d52 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java > 276ecb5 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java > 23860a3 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/ParallelizationInfo.java > 75a009e > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java > f8d1803 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Stats.java > e61b38f > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/StatsCollector.java > 1f56556 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/ExcessiveExchangeIdentifier.java > a237014 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/RecordDataType.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoDataType.java > c1e64e6 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/StaticDrillTable.java > c1e8dd1 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemRecordReader.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemRecords.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTable.java > 0bf2156 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java > a1bec1e > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTablePlugin.java > 2c70fd4 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTablePluginConfig.java > 93fe68e > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableScan.java > cdd0d18 > > exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestSystemTable.java > c1803bc > > Diff: https://reviews.apache.org/r/31938/diff/ > > > Testing > ------- > > Tested on 3-node cluster and in embedded mode. > ``` > > select * from sys.memory; > +------------+--------------+------------+---------------------+ > | host_name | total_memory | heap_size | direct_alloc_memory | > +------------+--------------+------------+---------------------+ > | perfnode206.perf.lab:31010 | 1073741824 | 395823432 | 5000000 > | > | perfnode208.perf.lab:31010 | 1073741824 | 337127496 | 2000000 > | > | perfnode207.perf.lab:31010 | 1073741824 | 289272760 | 2000000 > | > +------------+--------------+------------+---------------------+ > ``` > > > Thanks, > > Sudheesh Katkam > >
