[ 
https://issues.apache.org/jira/browse/PHOENIX-1264?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14148360#comment-14148360
 ] 

James Taylor commented on PHOENIX-1264:
---------------------------------------

Thanks for the patch, [~ramkrishna]. +1 after addressing this minor feedback:
- Instead of changing the default in individual sources (as shown below), 
change the default across the board for tests for 
QueryServicesTestImpl.DEFAULT_HISTOGRAM_BYTE_DEPTH.
{code}
+    @BeforeClass
+    @Shadower(classBeingShadowed = BaseHBaseManagedTimeIT.class)
+    public static void doSetup() throws Exception {
+        Map<String,String> props = Maps.newHashMapWithExpectedSize(3);
+        // Must update config before starting server
+        props.put(QueryServices.HISTOGRAM_BYTE_DEPTH_ATTRIB, 
Long.toString(20l));
+        props.put(QueryServices.QUEUE_SIZE_ATTRIB, Integer.toString(20));
+        props.put(QueryServices.THREAD_POOL_SIZE_ATTRIB, Integer.toString(20));
+        setUpTestDriver(new ReadOnlyProps(props.entrySet().iterator()));
+    }
+    
{code}
- Minor nit: how about "_AnalyzeTable" instead of "_ANALYZE" here so it's more 
like the others?
{code}
core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java
index b2e2806..b7b9b01 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java
@@ -60,6 +60,7 @@ abstract public class BaseScannerRegionObserver extends 
BaseRegionObserver {
     public static final String VIEW_CONSTANTS = "_ViewConstants";
     public static final String EXPECTED_UPPER_REGION_KEY = 
"_ExpectedUpperRegionKey";
     public static final String REVERSE_SCAN = "_ReverseScan";
+    public static final String ANALYZE = "_ANALYZE";
{code}
- Can we cover these checks with a new ScanUtil.isAnalyzeTable(Scan scan) call 
instead?
{code}
+        if (scan.getAttribute((BaseScannerRegionObserver.ANALYZE)) == null) {
{code}
- This call in MetaDataClient shouldn't be necessary, as the next time the 
table is accessed, it'll automatically be updated (b/c it's out-of-date now 
based on the clearCache call):
{code}
             updateCache(table.getSchemaName().getString(), 
table.getTableName().getString(), true);
-            return new MutationState(1, connection);
{code}
- Based on the issue you've uncovered (that stats must be updated completely 
for a region), there's a bit of follow on work needed if an ANALYZE is done on 
a tenant-specific table. This case will be optimized to only scan and analyze 
the current tenant's data, however we have to make sure that the entire 
region(s) containing that tenant is scanned (or we'll end up replacing the 
stats for that region with just the one we calculated for that tenant). I 
created PHOENIX-1296 as follow up work for this.

> Add StatisticsCollector to existing tables on first connection to cluster
> -------------------------------------------------------------------------
>
>                 Key: PHOENIX-1264
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1264
>             Project: Phoenix
>          Issue Type: Sub-task
>            Reporter: James Taylor
>            Assignee: ramkrishna.s.vasudevan
>         Attachments: Phoenix-1264_1.patch, Phoenix-1264_2.patch
>
>
> We'll need to add the StatisticsCollector to existing tables so that stats 
> are kept for these tables. We typically do that on first connection to the 
> cluster in ensureTableCreated when our system table is created by walking 
> through the existing Phoenix tables. We only need to do it if the system 
> table already exists (otherwise, it's a new installation).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to