Github user ChinmaySKulkarni commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/295#discussion_r178961071
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
 ---
    @@ -2643,6 +2661,26 @@ public void upgradeSystemTables(final String url, 
final Properties props) throws
                 metaConnection.setRunningUpgrade(true);
                 try {
                     
metaConnection.createStatement().executeUpdate(QueryConstants.CREATE_TABLE_METADATA);
    +
    +                // HBase Namespace SYSTEM is created by {@link 
ensureSystemTablesMigratedToSystemNamespace(ReadOnlyProps)} method
    +                // This statement will create its entry in SYSCAT table, 
so that GRANT/REVOKE commands can work
    +                // with SYSTEM Namespace. (See PHOENIX-4227 
https://issues.apache.org/jira/browse/PHOENIX-4227)
    +                if (SchemaUtil.isNamespaceMappingEnabled(PTableType.SYSTEM,
    +                  ConnectionQueryServicesImpl.this.getProps())) {
    --- End diff --
    
    @JamesRTaylor Makes sense. Note that we don't **already** have the SYSMUTEX 
lock at this point (referring to your earlier comment). This is in fact 
necessary to avoid an _UpgradeInProgressException_ from 
_ensureSystemTablesMigratedToSystemNamespace_ since we try to acquire the 
SYSMUTEX lock inside here as well. So, moving 
_ensureSystemTablesMigratedToSystemNamespace_ to this place shouldn't lead to 
any problems.


---

Reply via email to