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