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

ASF GitHub Bot commented on PHOENIX-6141:
-----------------------------------------

jpisaac commented on code in PR #1575:
URL: https://github.com/apache/phoenix/pull/1575#discussion_r1175848743


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ChildLinkMetaDataEndpoint.java:
##########
@@ -111,4 +145,162 @@ private PhoenixMetaDataCoprocessorHost 
getCoprocessorHost() {
                return phoenixAccessCoprocessorHost;
        }
 
+    /**
+     * Class that verifies a given row of a SYSTEM.CHILD_LINK table.
+     * An instance of this class is created for each scanner on the table
+     * and used to verify individual rows.
+     */
+    public class ChildLinkMetaDataScanner extends ReadRepairScanner {
+
+        private Table sysCatHTable;
+        private Scan childLinkScan;
+
+        public ChildLinkMetaDataScanner(RegionCoprocessorEnvironment env,
+                                        Scan scan,
+                                        RegionScanner scanner) throws 
IOException {
+            super(env, scan, scanner);
+            ageThreshold = env.getConfiguration().getLong(
+                    
QueryServices.CHILD_LINK_ROW_AGE_THRESHOLD_TO_DELETE_MS_ATTRIB,
+                    
QueryServicesOptions.DEFAULT_CHILD_LINK_ROW_AGE_THRESHOLD_TO_DELETE_MS);
+            sysCatHTable = ServerUtil.ConnectionFactory.
+                    
getConnection(ServerUtil.ConnectionType.DEFAULT_SERVER_CONNECTION, env).
+                    
getTable(TableName.valueOf(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME));
+        }
+
+        /*
+        If the row is VERIFIED, remove the empty column from the row
+         */
+        @Override
+        public boolean verifyRow(List<Cell> cellList) {
+            long cellListSize = cellList.size();
+            Cell cell = null;
+            if (cellListSize == 0) {
+                return true;
+            }
+            Iterator<Cell> cellIterator = cellList.iterator();
+            while (cellIterator.hasNext()) {
+                cell = cellIterator.next();
+                if (isEmptyColumn(cell)) {
+                    if (Bytes.compareTo(cell.getValueArray(), 
cell.getValueOffset(), cell.getValueLength(),
+                            VERIFIED_BYTES, 0, VERIFIED_BYTES.length) != 0) {
+                        return false;
+                    }
+                    // Empty column is not supposed to be returned to the 
client except
+                    // when it is the only column included in the scan
+                    if (cellListSize > 1) {
+                        cellIterator.remove();
+                    }
+                    return true;
+                }
+            }
+            // no empty column found
+            return false;
+        }
+
+        /*
+        Find parent link in syscat for given child link.
+        If found, mark child link row VERIFIED and start a new scan from it.
+        Otherwise, delete if row is old enough.
+         */
+        @Override
+        public void repairRow(List<Cell> row) throws IOException {
+            Cell cell = row.get(0);
+            byte[] rowKey = CellUtil.cloneRow(cell);
+            long ts = row.get(0).getTimestamp();
+
+            Scan sysCatScan = getExternalScanner();
+            childLinkScan = new Scan(scan);
+
+
+            // build syscat rowKey using given rowKey
+            byte[] sysCatRowKey = getSysCatRowKey(rowKey);

Review Comment:
   We should check for the existence of the view header too. The existence of 
only child->parent link type (3) row might be an bug.





> Ensure consistency between SYSTEM.CATALOG and SYSTEM.CHILD_LINK
> ---------------------------------------------------------------
>
>                 Key: PHOENIX-6141
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-6141
>             Project: Phoenix
>          Issue Type: Improvement
>    Affects Versions: 5.0.0, 4.15.0
>            Reporter: Chinmay Kulkarni
>            Assignee: Palash Chauhan
>            Priority: Blocker
>             Fix For: 5.2.0, 5.1.4
>
>
> Before 4.15, "CREATE/DROP VIEW" was an atomic operation since we were issuing 
> batch mutations on just the 1 SYSTEM.CATALOG region. In 4.15 we introduced 
> SYSTEM.CHILD_LINK to store the parent->child links and so a CREATE VIEW is no 
> longer atomic since it consists of 2 separate RPCs  (1 to SYSTEM.CHILD_LINK 
> to add the linking row and another to SYSTEM.CATALOG to write metadata for 
> the new view). 
> If the second RPC i.e. the RPC to write metadata to SYSTEM.CATALOG fails 
> after the 1st RPC has already gone through, there will be an inconsistency 
> between both metadata tables. We will see orphan parent->child linking rows 
> in SYSTEM.CHILD_LINK in this case. This can cause the following issues:
> # ALTER TABLE calls on the base table will fail
> # DROP TABLE without CASCADE will fail
> # The upgrade path has calls like UpgradeUtil.upgradeTable() which will fail
> # Any metadata consistency checks can be thrown off
> # Unnecessary extra storage of orphan links
> The first 3 issues happen because we wrongly deduce that a base table has 
> child views due to the orphan linking rows.
> This Jira aims at trying to come up with a way to make mutations among 
> SYSTEM.CATALOG and SYSTEM.CHILD_LINK an atomic transaction. We can use a 
> 2-phase commit approach like in global indexing or also potentially explore 
> using a transaction manager. 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to