> On March 11, 2015, 10:22 p.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemRecords.java, > > line 48 > > <https://reviews.apache.org/r/31938/diff/1/?file=891400#file891400line48> > > > > 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.
I am just curious here. The rationale was to have the collection of implementations of SystemRecord in one place. And access to the singleton of each implementation is only through SystemRecords e.g. SystemRecords.getMemoryRecord() and not through individual classes e.g. MemoryRecord.getInstance(). Is there a problem with this approach? > On March 11, 2015, 10:22 p.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java, > > line 62 > > <https://reviews.apache.org/r/31938/diff/1/?file=891402#file891402line62> > > > > Could this be a method on the SystemTable enum instead? It's possible but again, I am curious here. I thought it would be awkward to have: switch(this) { case OPTION: ... } (once for local tables and once for distributed tables) > On March 11, 2015, 10:22 p.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java, > > line 77 > > <https://reviews.apache.org/r/31938/diff/1/?file=891402#file891402line77> > > > > Could this be a method on the SystemTable enum instead? ^ Same as above. On March 11, 2015, 10:22 p.m., Sudheesh Katkam wrote: > > 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. It does. Should I remove the port altogether? Yes) This way users can join with sys.drillbits table. No) Then, should I list user port, control port and data port? - Sudheesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31938/#review76138 ----------------------------------------------------------- On March 11, 2015, 5:04 p.m., Sudheesh Katkam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31938/ > ----------------------------------------------------------- > > (Updated March 11, 2015, 5:04 p.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 > >
