You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2018/01/12 00:30:35 UTC
[1/2] phoenix git commit: PHOENIX-4525 Integer overflow in GroupBy
execution
Repository: phoenix
Updated Branches:
refs/heads/4.13-cdh5.11.2 5a02ef439 -> f59db65bc
PHOENIX-4525 Integer overflow in GroupBy execution
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/a77ce3f0
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/a77ce3f0
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/a77ce3f0
Branch: refs/heads/4.13-cdh5.11.2
Commit: a77ce3f09cae0e223ee50ac92ac3e1890115d98c
Parents: 5a02ef4
Author: Sergey Soldatov <ss...@apache.org>
Authored: Wed Jan 10 13:04:00 2018 -0800
Committer: James Taylor <jt...@salesforce.com>
Committed: Thu Jan 11 16:14:50 2018 -0800
----------------------------------------------------------------------
.../main/java/org/apache/phoenix/util/SizedUtil.java | 2 +-
.../org/apache/phoenix/memory/MemoryManagerTest.java | 15 +++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/phoenix/blob/a77ce3f0/phoenix-core/src/main/java/org/apache/phoenix/util/SizedUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/SizedUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/SizedUtil.java
index f82c1b8..d67ed7f 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/SizedUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/SizedUtil.java
@@ -67,7 +67,7 @@ public class SizedUtil {
public static long sizeOfMap(int nRows, int keySize, int valueSize) {
return SizedUtil.OBJECT_SIZE * 4 + sizeOfArrayList(nRows) /* key set */ + nRows * (
- SizedUtil.MAP_ENTRY_SIZE + /* entry set */
+ SizedUtil.MAP_ENTRY_SIZE * 1L + /* entry set */
keySize + // key size
valueSize); // value size
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/a77ce3f0/phoenix-core/src/test/java/org/apache/phoenix/memory/MemoryManagerTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/memory/MemoryManagerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/memory/MemoryManagerTest.java
index 6da2526..897bb5b 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/memory/MemoryManagerTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/memory/MemoryManagerTest.java
@@ -26,6 +26,7 @@ import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.phoenix.coprocessor.GroupedAggregateRegionObserver;
import org.apache.phoenix.memory.MemoryManager.MemoryChunk;
import org.junit.Test;
@@ -177,4 +178,18 @@ public class MemoryManagerTest {
// make sure all memory is freed
assertTrue(gmm.getAvailableMemory() == gmm.getMaxMemory());
}
+
+ /**
+ * Test for SpillableGroupByCache which is using MemoryManager to allocate chunks for GroupBy execution
+ * @throws Exception
+ */
+ @Test
+ public void testCorrectnessOfChunkAllocation() throws Exception {
+ for(int i = 1000;i < Integer.MAX_VALUE;) {
+ i *=1.5f;
+ long result = GroupedAggregateRegionObserver.sizeOfUnorderedGroupByMap(i, 100);
+ assertTrue("Size for GroupByMap is negative" , result > 0);
+ }
+ }
+
}
[2/2] phoenix git commit: PHOENIX-4523
phoenix.schema.isNamespaceMappingEnabled problem (Karan Mehta)
Posted by ja...@apache.org.
PHOENIX-4523 phoenix.schema.isNamespaceMappingEnabled problem (Karan Mehta)
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/f59db65b
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/f59db65b
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/f59db65b
Branch: refs/heads/4.13-cdh5.11.2
Commit: f59db65bcdbf2ed0b783917112843a9c37f49a41
Parents: a77ce3f
Author: James Taylor <jt...@salesforce.com>
Authored: Thu Jan 11 16:22:09 2018 -0800
Committer: James Taylor <jt...@salesforce.com>
Committed: Thu Jan 11 16:29:11 2018 -0800
----------------------------------------------------------------------
.../query/ConnectionQueryServicesImpl.java | 32 +++++++++++---------
.../org/apache/phoenix/util/UpgradeUtil.java | 2 ++
.../query/ConnectionQueryServicesImplTest.java | 6 ++--
3 files changed, 22 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/phoenix/blob/f59db65b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index 9df93b6..35b85e4 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -2523,15 +2523,15 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
}
}
- void createSysMutexTable(HBaseAdmin admin, ReadOnlyProps props) throws IOException, SQLException {
+ void createSysMutexTableIfNotExists(HBaseAdmin admin, ReadOnlyProps props) throws IOException, SQLException {
try {
- final TableName mutexTableName = SchemaUtil.getPhysicalTableName(
- PhoenixDatabaseMetaData.SYSTEM_MUTEX_NAME, props);
- List<TableName> systemTables = getSystemTableNames(admin);
- if (systemTables.contains(mutexTableName)) {
+ if(admin.tableExists(PhoenixDatabaseMetaData.SYSTEM_MUTEX_NAME) || admin.tableExists(TableName.valueOf(
+ PhoenixDatabaseMetaData.SYSTEM_SCHEMA_NAME,PhoenixDatabaseMetaData.SYSTEM_MUTEX_TABLE_NAME))) {
logger.debug("System mutex table already appears to exist, not creating it");
return;
}
+ final TableName mutexTableName = SchemaUtil.getPhysicalTableName(
+ PhoenixDatabaseMetaData.SYSTEM_MUTEX_NAME, props);
HTableDescriptor tableDesc = new HTableDescriptor(mutexTableName);
HColumnDescriptor columnDesc = new HColumnDescriptor(
PhoenixDatabaseMetaData.SYSTEM_MUTEX_FAMILY_NAME_BYTES);
@@ -2547,10 +2547,17 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
}
} catch (TableExistsException e) {
// Ignore
+ } catch (IOException e) {
+ if(!Iterables.isEmpty(Iterables.filter(Throwables.getCausalChain(e), AccessDeniedException.class)) ||
+ !Iterables.isEmpty(Iterables.filter(Throwables.getCausalChain(e), org.apache.hadoop.hbase.TableNotFoundException.class))) {
+ // Ignore
+ } else {
+ throw e;
+ }
}
}
- List<TableName> getSystemTableNames(HBaseAdmin admin) throws IOException {
+ List<TableName> getSystemTableNamesInDefaultNamespace(HBaseAdmin admin) throws IOException {
return Lists.newArrayList(admin.listTableNames(QueryConstants.SYSTEM_SCHEMA_NAME + "\\..*"));
}
@@ -2569,7 +2576,7 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
// Catch the IOException to log the error message and then bubble it up for the client to retry.
try {
- createSysMutexTable(hbaseAdmin, ConnectionQueryServicesImpl.this.getProps());
+ createSysMutexTableIfNotExists(hbaseAdmin, ConnectionQueryServicesImpl.this.getProps());
} catch (IOException exception) {
logger.error("Failed to created SYSMUTEX table. Upgrade or migration is not possible without it. Please retry.");
throw exception;
@@ -2621,7 +2628,7 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
!SchemaUtil.isNamespaceMappingEnabled(PTableType.SYSTEM,
ConnectionQueryServicesImpl.this.getProps())) {
try (HBaseAdmin admin = getAdmin()) {
- createSysMutexTable(admin, this.getProps());
+ createSysMutexTableIfNotExists(admin, this.getProps());
}
}
if (acquiredMutexLock = acquireUpgradeMutex(currentServerSideTableTimeStamp, mutexRowKey)) {
@@ -3164,7 +3171,7 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
// below. If the NS does exist and is mapped, the below check will exit gracefully.
}
- List<TableName> tableNames = getSystemTableNames(admin);
+ List<TableName> tableNames = getSystemTableNamesInDefaultNamespace(admin);
// No tables exist matching "SYSTEM\..*", they are all already in "SYSTEM:.*"
if (tableNames.size() == 0) { return; }
// Try to move any remaining tables matching "SYSTEM\..*" into "SYSTEM:"
@@ -3176,12 +3183,7 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
// If we cannot acquire lock, it means some old client is either migrating SYSCAT or trying to upgrade the
// schema of SYSCAT table and hence it should not be interrupted
// Create mutex if not already created
- if (!tableNames.contains(PhoenixDatabaseMetaData.SYSTEM_MUTEX_HBASE_TABLE_NAME)) {
- TableName mutexName = SchemaUtil.getPhysicalTableName(PhoenixDatabaseMetaData.SYSTEM_MUTEX_NAME, props);
- if (PhoenixDatabaseMetaData.SYSTEM_MUTEX_HBASE_TABLE_NAME.equals(mutexName) || !tableNames.contains(mutexName)) {
- createSysMutexTable(admin, props);
- }
- }
+ createSysMutexTableIfNotExists(admin, props);
acquiredMutexLock = acquireUpgradeMutex(MetaDataProtocol.MIN_SYSTEM_TABLE_MIGRATION_TIMESTAMP, mutexRowKey);
if(acquiredMutexLock) {
logger.debug("Acquired lock in SYSMUTEX table for migrating SYSTEM tables to SYSTEM namespace");
http://git-wip-us.apache.org/repos/asf/phoenix/blob/f59db65b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
index 4488e86..c4c2834 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
@@ -1779,6 +1779,8 @@ public class UpgradeUtil {
admin.deleteTable(srcTableName);
logger.info(String.format("deleting snapshot %s..", snapshotName));
admin.deleteSnapshot(snapshotName);
+ } else {
+ logger.info(String.format("Destination Table %s already exists. No migration needed.", destTableName));
}
}
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/f59db65b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
index 4708ffb..b5c3e4a 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
@@ -49,10 +49,10 @@ public class ConnectionQueryServicesImplTest {
when(cqs.createSchema(any(List.class), anyString())).thenCallRealMethod();
doCallRealMethod().when(cqs).ensureSystemTablesMigratedToSystemNamespace(any(ReadOnlyProps.class));
// Do nothing for this method, just check that it was invoked later
- doNothing().when(cqs).createSysMutexTable(any(HBaseAdmin.class), any(ReadOnlyProps.class));
+ doNothing().when(cqs).createSysMutexTableIfNotExists(any(HBaseAdmin.class), any(ReadOnlyProps.class));
// Spoof out this call so that ensureSystemTablesUpgrade() will return-fast.
- when(cqs.getSystemTableNames(any(HBaseAdmin.class))).thenReturn(Collections.<TableName> emptyList());
+ when(cqs.getSystemTableNamesInDefaultNamespace(any(HBaseAdmin.class))).thenReturn(Collections.<TableName> emptyList());
// Throw a special exception to check on later
doThrow(PHOENIX_IO_EXCEPTION).when(cqs).ensureNamespaceCreated(anyString());
@@ -64,7 +64,7 @@ public class ConnectionQueryServicesImplTest {
// Should be called after upgradeSystemTables()
// Proves that execution proceeded
- verify(cqs).getSystemTableNames(any(HBaseAdmin.class));
+ verify(cqs).getSystemTableNamesInDefaultNamespace(any(HBaseAdmin.class));
try {
// Verifies that the exception is propagated back to the caller