You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by BinShi-SecularBird <gi...@git.apache.org> on 2018/12/15 00:37:48 UTC
[GitHub] phoenix pull request #416: PHOENIX-5069 please go to JIRA to see the detaile...
GitHub user BinShi-SecularBird opened a pull request:
https://github.com/apache/phoenix/pull/416
PHOENIX-5069 please go to JIRA to see the detailed design document at…
…tached.
PHOENIX-4998 The estimated instance size of GuidePostsInfo doesn't count the last update timestamp array.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/BinShi-SecularBird/phoenix PHOENIX-5069
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/phoenix/pull/416.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #416
----
commit 69801e540246d998d02abf90705edc3dd5835dcc
Author: Bin <bs...@...>
Date: 2018-10-28T19:21:27Z
PHOENIX-5069 please go to JIRA to see the detailed design document attached.
PHOENIX-4998 The estimated instance size of GuidePostsInfo doesn't count the last update timestamp array.
----
---
[GitHub] phoenix issue #416: PHOENIX-5069 please go to JIRA to see the detailed desig...
Posted by BinShi-SecularBird <gi...@git.apache.org>.
Github user BinShi-SecularBird commented on the issue:
https://github.com/apache/phoenix/pull/416
@karanmehta93
---
[GitHub] phoenix pull request #416: PHOENIX-5069 please go to JIRA to see the detaile...
Posted by twdsilva <gi...@git.apache.org>.
Github user twdsilva commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/416#discussion_r245172433
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/query/PhoenixStatsCacheLoaderTest.java ---
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.query;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.LoadingCache;
+import com.google.common.cache.Weigher;
+import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.phoenix.schema.stats.GuidePostsInfo;
+import org.apache.phoenix.schema.stats.GuidePostsKey;
+import org.apache.phoenix.util.ByteUtil;
+import org.junit.Test;
+
+import java.lang.Thread;
+import java.util.Collections;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.ExecutorService;
+
+/**
+ * Test class around the PhoenixStatsCacheLoader.
+ */
+public class PhoenixStatsCacheLoaderTest {
+ /**
+ * {@link PhoenixStatsLoader} test implementation for the Stats Loader.
+ */
+ protected class TestStatsLoaderImpl implements PhoenixStatsLoader {
+ private int maxLength = 1;
+
+ @Override
+ public boolean needsLoad() {
+ // Whenever it's called, we try to load stats from stats table
+ // no matter it has been updated or not.
+ return true;
+ }
+
+ @Override
+ public GuidePostsInfo loadStats(GuidePostsKey statsKey, GuidePostsInfo prevGuidepostInfo) throws Exception {
+ try {
+ Thread.sleep(2000);
+ }
+ catch (InterruptedException e) {
+ assertFalse(true);
+ }
+
+ return new GuidePostsInfo(Collections.<Long> emptyList(),
+ new ImmutableBytesWritable(ByteUtil.EMPTY_BYTE_ARRAY),
+ Collections.<Long> emptyList(), maxLength++, 0, Collections.<Long> emptyList());
+ }
+ }
+
+ GuidePostsInfo getStats(LoadingCache<GuidePostsKey, GuidePostsInfo> cache, GuidePostsKey guidePostsKey) {
+ GuidePostsInfo guidePostsInfo;
+ try {
+ guidePostsInfo = cache.get(guidePostsKey);
+ } catch (ExecutionException e) {
+ assertFalse(true);
+ return GuidePostsInfo.NO_GUIDEPOST;
+ }
+
+ return guidePostsInfo;
+ }
+
+ void sleep(int x) {
+ try {
+ Thread.sleep(x);
+ }
+ catch (InterruptedException e) {
+ assertFalse(true);
+ }
+ }
+
+ @Test
+ public void testStatsBeingAutomaticallyRefreshed() {
+ ExecutorService executor = Executors.newFixedThreadPool(4);
+
+ LoadingCache<GuidePostsKey, GuidePostsInfo> cache = CacheBuilder.newBuilder()
+ // Refresh entries a given amount of time after they were written
+ .refreshAfterWrite(1000, TimeUnit.MILLISECONDS)
+ // Maximum total weight (size in bytes) of stats entries
+ .maximumWeight(QueryServicesOptions.DEFAULT_STATS_MAX_CACHE_SIZE)
+ // Defer actual size to the PTableStats.getEstimatedSize()
+ .weigher(new Weigher<GuidePostsKey, GuidePostsInfo>() {
+ @Override public int weigh(GuidePostsKey key, GuidePostsInfo info) {
+ return info.getEstimatedSize();
+ }
+ })
+ // Log removals at TRACE for debugging
+ .removalListener(new GuidePostsCache.PhoenixStatsCacheRemovalListener())
+ // Automatically load the cache when entries are missing
+ .build(new PhoenixStatsCacheLoader(new TestStatsLoaderImpl(), executor));
+
+ GuidePostsKey guidePostsKey = new GuidePostsKey(new byte[4], new byte[4]);
+ GuidePostsInfo guidePostsInfo = getStats(cache, guidePostsKey);
+ assertTrue(guidePostsInfo.getMaxLength() == 1);
+ sleep(3000);
--- End diff --
Instead of relying on sleep which tends to cause tests to fail on the build machines, can you use a latch or some other mechanism to determine that the guideposts have been refreshed?
---
[GitHub] phoenix pull request #416: PHOENIX-5069 please go to JIRA to see the detaile...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/416#discussion_r244856169
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/GuidePostsCache.java ---
@@ -80,38 +94,46 @@ public GuidePostsCache(ConnectionQueryServices queryServices, Configuration conf
// Log removals at TRACE for debugging
.removalListener(new PhoenixStatsCacheRemovalListener())
// Automatically load the cache when entries are missing
- .build(new StatsLoader());
+ .build(new PhoenixStatsCacheLoader(new StatsLoaderImpl(), executor));
}
/**
- * {@link CacheLoader} implementation for the Phoenix Table Stats cache.
+ * {@link PhoenixStatsLoader} implementation for the Stats Loader.
*/
- protected class StatsLoader extends CacheLoader<GuidePostsKey, GuidePostsInfo> {
+ protected class StatsLoaderImpl implements PhoenixStatsLoader {
+ @Override
+ public boolean needsLoad() {
+ // Whenever it's called, we try to load stats from stats table
+ // no matter it has been updated or not.
+ return true;
+ }
+
@Override
- public GuidePostsInfo load(GuidePostsKey statsKey) throws Exception {
+ public GuidePostsInfo loadStats(GuidePostsKey statsKey, GuidePostsInfo prevGuidepostInfo) throws Exception {
@SuppressWarnings("deprecation")
- Table statsHTable = queryServices.getTable(SchemaUtil.getPhysicalName(
+ TableName tableName = SchemaUtil.getPhysicalName(
PhoenixDatabaseMetaData.SYSTEM_STATS_NAME_BYTES,
- queryServices.getProps()).getName());
+ queryServices.getProps());
+ Table statsHTable = queryServices.getTable(tableName.getName());
try {
GuidePostsInfo guidePostsInfo = StatisticsUtil.readStatistics(statsHTable, statsKey,
HConstants.LATEST_TIMESTAMP);
traceStatsUpdate(statsKey, guidePostsInfo);
return guidePostsInfo;
} catch (TableNotFoundException e) {
// On a fresh install, stats might not yet be created, don't warn about this.
- logger.debug("Unable to locate Phoenix stats table", e);
- return GuidePostsInfo.NO_GUIDEPOST;
+ logger.debug("Unable to locate Phoenix stats table: " + tableName.toString(), e);
+ return prevGuidepostInfo;
} catch (IOException e) {
- logger.warn("Unable to read from stats table", e);
+ logger.warn("Unable to read from stats table: " + tableName.toString(), e);
// Just cache empty stats. We'll try again after some time anyway.
--- End diff --
nit: remove this comment.
---
[GitHub] phoenix pull request #416: PHOENIX-5069 please go to JIRA to see the detaile...
Posted by BinShi-SecularBird <gi...@git.apache.org>.
Github user BinShi-SecularBird closed the pull request at:
https://github.com/apache/phoenix/pull/416
---