drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sudheesh Katkam" <skat...@maprtech.com>
Subject Re: Review Request 31938: DRILL-2275: Need implementations of sys tables for drill memory and threads profiles
Date Wed, 11 Mar 2015 23:38:16 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message