You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "shahrs87 (via GitHub)" <gi...@apache.org> on 2023/10/05 03:48:40 UTC

[PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

shahrs87 opened a new pull request, #1702:
URL: https://github.com/apache/phoenix/pull/1702

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355858128


##########
phoenix-core/src/test/java/org/apache/phoenix/cache/ServerMetadataCacheTest.java:
##########
@@ -420,6 +423,126 @@ public void testInvalidateCacheForBaseTableWithUpdateIndexStatement() throws Exc
     }
 
 
+    /**
+     *  Test that we invalidate the cache for parent table and update the last ddl timestamp
+     *  of the parent table while we add an index.
+     *  Test that we invalidate the cache for parent table and index when we drop an index.
+     *  Also we update the last ddl timestamp for parent table when we drop an index.
+     * @throws Exception
+     */
+    @Test
+    public void testUpdateLastDDLTimestampTableAfterIndexCreation() throws Exception {
+        String tableName = generateUniqueName();
+        byte[] tableNameBytes = Bytes.toBytes(tableName);
+        String indexName = generateUniqueName();
+        byte[] indexNameBytes = Bytes.toBytes(indexName);
+        ServerMetadataCache cache = ServerMetadataCache.getInstance(config);
+        String ddl =
+                "create table  " + tableName + " ( k integer PRIMARY KEY," + " v1 integer,"
+                        + " v2 integer)";
+        String createIndexDDL = "create index  " + indexName + " on " + tableName + " (v1)";
+        String dropIndexDDL = "DROP INDEX " + indexName + " ON " + tableName;
+        try (Connection conn = DriverManager.getConnection(getUrl());
+             Statement stmt = conn.createStatement()) {
+            conn.setAutoCommit(true);
+            stmt.execute(ddl);
+            long tableLastDDLTimestampBeforeIndexCreation = getLastDDLTimestamp(tableName);
+            // Populate the cache
+            assertNotNull(cache.getLastDDLTimestampForTable(null, null, tableNameBytes));
+            Thread.sleep(1);

Review Comment:
   We are generating the last ddl timestamp using `EnvironmentEdgeManager.currentTimeMillis()` when we create/drop an index. I just want to be very sure that we don't create the same time when we created an index as well as when we dropped an index. So I sleep for 1 milliseconds in between.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#issuecomment-1755631442

   @tkhurana @palashc  Can you please help review the patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355870002


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3507,23 +3525,30 @@ private void invalidateServerMetadataCacheWithRetries(Admin admin,
                     // DDL operations. We also need to think of we need separate RPC handler
                     // threads for this?
                     ServerRpcController controller = new ServerRpcController();
+                    for (InvalidateServerMetadataCacheRequest invalidateCacheRequest:

Review Comment:
   But then it won't have the region server name in getRequest method. Having a regionserver name would be helpful.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 merged PR #1702:
URL: https://github.com/apache/phoenix/pull/1702


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355874777


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3507,23 +3525,30 @@ private void invalidateServerMetadataCacheWithRetries(Admin admin,
                     // DDL operations. We also need to think of we need separate RPC handler
                     // threads for this?
                     ServerRpcController controller = new ServerRpcController();
+                    for (InvalidateServerMetadataCacheRequest invalidateCacheRequest:

Review Comment:
   Yes, in the process of making that change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355860010


##########
phoenix-core/src/test/java/org/apache/phoenix/cache/ServerMetadataCacheTest.java:
##########
@@ -420,6 +423,126 @@ public void testInvalidateCacheForBaseTableWithUpdateIndexStatement() throws Exc
     }
 
 
+    /**
+     *  Test that we invalidate the cache for parent table and update the last ddl timestamp
+     *  of the parent table while we add an index.
+     *  Test that we invalidate the cache for parent table and index when we drop an index.
+     *  Also we update the last ddl timestamp for parent table when we drop an index.
+     * @throws Exception
+     */
+    @Test
+    public void testUpdateLastDDLTimestampTableAfterIndexCreation() throws Exception {
+        String tableName = generateUniqueName();
+        byte[] tableNameBytes = Bytes.toBytes(tableName);
+        String indexName = generateUniqueName();
+        byte[] indexNameBytes = Bytes.toBytes(indexName);
+        ServerMetadataCache cache = ServerMetadataCache.getInstance(config);
+        String ddl =
+                "create table  " + tableName + " ( k integer PRIMARY KEY," + " v1 integer,"
+                        + " v2 integer)";
+        String createIndexDDL = "create index  " + indexName + " on " + tableName + " (v1)";
+        String dropIndexDDL = "DROP INDEX " + indexName + " ON " + tableName;
+        try (Connection conn = DriverManager.getConnection(getUrl());
+             Statement stmt = conn.createStatement()) {
+            conn.setAutoCommit(true);
+            stmt.execute(ddl);
+            long tableLastDDLTimestampBeforeIndexCreation = getLastDDLTimestamp(tableName);
+            // Populate the cache
+            assertNotNull(cache.getLastDDLTimestampForTable(null, null, tableNameBytes));
+            Thread.sleep(1);

Review Comment:
   Got it. Will be nice to add a comment mentioning this. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355844117


##########
phoenix-core/src/test/java/org/apache/phoenix/cache/ServerMetadataCacheTest.java:
##########
@@ -420,6 +423,126 @@ public void testInvalidateCacheForBaseTableWithUpdateIndexStatement() throws Exc
     }
 
 
+    /**
+     *  Test that we invalidate the cache for parent table and update the last ddl timestamp
+     *  of the parent table while we add an index.
+     *  Test that we invalidate the cache for parent table and index when we drop an index.
+     *  Also we update the last ddl timestamp for parent table when we drop an index.
+     * @throws Exception
+     */
+    @Test
+    public void testUpdateLastDDLTimestampTableAfterIndexCreation() throws Exception {
+        String tableName = generateUniqueName();
+        byte[] tableNameBytes = Bytes.toBytes(tableName);
+        String indexName = generateUniqueName();
+        byte[] indexNameBytes = Bytes.toBytes(indexName);
+        ServerMetadataCache cache = ServerMetadataCache.getInstance(config);
+        String ddl =
+                "create table  " + tableName + " ( k integer PRIMARY KEY," + " v1 integer,"
+                        + " v2 integer)";
+        String createIndexDDL = "create index  " + indexName + " on " + tableName + " (v1)";
+        String dropIndexDDL = "DROP INDEX " + indexName + " ON " + tableName;
+        try (Connection conn = DriverManager.getConnection(getUrl());
+             Statement stmt = conn.createStatement()) {
+            conn.setAutoCommit(true);
+            stmt.execute(ddl);
+            long tableLastDDLTimestampBeforeIndexCreation = getLastDDLTimestamp(tableName);
+            // Populate the cache
+            assertNotNull(cache.getLastDDLTimestampForTable(null, null, tableNameBytes));
+            Thread.sleep(1);

Review Comment:
   For my understanding, why is sleep required in these tests? 



##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3507,23 +3525,30 @@ private void invalidateServerMetadataCacheWithRetries(Admin admin,
                     // DDL operations. We also need to think of we need separate RPC handler
                     // threads for this?
                     ServerRpcController controller = new ServerRpcController();
+                    for (InvalidateServerMetadataCacheRequest invalidateCacheRequest:

Review Comment:
   We are already iterating over the list of `InvalidateServerMetadataCacheRequests` in `getRequest` to create the proto request. Can we do the logging there? The list is very small so maybe it does not matter?



##########
phoenix-core/src/test/java/org/apache/phoenix/cache/ServerMetadataCacheTest.java:
##########
@@ -420,6 +423,126 @@ public void testInvalidateCacheForBaseTableWithUpdateIndexStatement() throws Exc
     }
 
 
+    /**
+     *  Test that we invalidate the cache for parent table and update the last ddl timestamp
+     *  of the parent table while we add an index.
+     *  Test that we invalidate the cache for parent table and index when we drop an index.
+     *  Also we update the last ddl timestamp for parent table when we drop an index.
+     * @throws Exception
+     */
+    @Test
+    public void testUpdateLastDDLTimestampTableAfterIndexCreation() throws Exception {
+        String tableName = generateUniqueName();
+        byte[] tableNameBytes = Bytes.toBytes(tableName);
+        String indexName = generateUniqueName();
+        byte[] indexNameBytes = Bytes.toBytes(indexName);
+        ServerMetadataCache cache = ServerMetadataCache.getInstance(config);
+        String ddl =
+                "create table  " + tableName + " ( k integer PRIMARY KEY," + " v1 integer,"
+                        + " v2 integer)";
+        String createIndexDDL = "create index  " + indexName + " on " + tableName + " (v1)";
+        String dropIndexDDL = "DROP INDEX " + indexName + " ON " + tableName;
+        try (Connection conn = DriverManager.getConnection(getUrl());
+             Statement stmt = conn.createStatement()) {
+            conn.setAutoCommit(true);
+            stmt.execute(ddl);
+            long tableLastDDLTimestampBeforeIndexCreation = getLastDDLTimestamp(tableName);
+            // Populate the cache
+            assertNotNull(cache.getLastDDLTimestampForTable(null, null, tableNameBytes));
+            Thread.sleep(1);
+            stmt.execute(createIndexDDL);
+            // Make sure that we have invalidated the last ddl timestamp for parent table
+            // on all regionservers after we create an index.
+            assertNull(cache.getLastDDLTimestampForTableFromCacheOnly(null, null, tableNameBytes));
+            long tableLastDDLTimestampAfterIndexCreation = getLastDDLTimestamp(tableName);
+            assertNotNull(tableLastDDLTimestampAfterIndexCreation);
+            assertTrue(tableLastDDLTimestampAfterIndexCreation >
+                    tableLastDDLTimestampBeforeIndexCreation);
+            long indexLastDDLTimestampAfterCreation = getLastDDLTimestamp(indexName);
+            // Make sure that last ddl timestamp is cached on the regionserver.
+            assertNotNull(indexLastDDLTimestampAfterCreation);
+            Thread.sleep(1);
+            stmt.execute(dropIndexDDL);
+            // Make sure that we invalidate the cache on regionserver for base table and an index
+            // after we dropped an index.
+            assertNull(cache.getLastDDLTimestampForTableFromCacheOnly(null, null, tableNameBytes));
+            assertNull(cache.getLastDDLTimestampForTableFromCacheOnly(null, null, indexNameBytes));
+            long tableLastDDLTimestampAfterIndexDeletion = getLastDDLTimestamp(tableName);
+            // Verify that last ddl timestamp after index deletion is greater than
+            // the previous last ddl timestamp.
+            assertNotNull(tableLastDDLTimestampAfterIndexDeletion);
+            assertTrue(tableLastDDLTimestampAfterIndexDeletion >
+                    tableLastDDLTimestampAfterIndexCreation);
+        }
+    }
+
+    /**
+     *  Test that we invalidate the cache of the immediate parent view

Review Comment:
   We are also testing the drop-index case so the comment can be updated. 



##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/InvalidateServerMetadataCacheRequest.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.coprocessor;
+
+public class InvalidateServerMetadataCacheRequest {

Review Comment:
   Maybe a `toString` method could be useful when we need to log the contents of this request?



##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/InvalidateServerMetadataCacheRequest.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.coprocessor;
+
+public class InvalidateServerMetadataCacheRequest {
+    private final byte[] tenantId;
+    private final byte[] schema;
+    private final byte[] name;
+
+    public InvalidateServerMetadataCacheRequest(byte[] tenantId, byte[] schema, byte[] name) {
+        this.tenantId = tenantId;
+        this.schema = schema;
+        this.name = name;

Review Comment:
   nit: Should these be `schemaName` and  `tableName` ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#issuecomment-1758694017

   Will wait for jenkins build before merging.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355873574


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3507,23 +3525,30 @@ private void invalidateServerMetadataCacheWithRetries(Admin admin,
                     // DDL operations. We also need to think of we need separate RPC handler
                     // threads for this?
                     ServerRpcController controller = new ServerRpcController();
+                    for (InvalidateServerMetadataCacheRequest invalidateCacheRequest:

Review Comment:
   Yeah that makes sense. In that case, do you think we can override `toString` in `InvalidateServerMetadataCacheRequest` to make it cleaner here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355867068


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3507,23 +3525,30 @@ private void invalidateServerMetadataCacheWithRetries(Admin admin,
                     // DDL operations. We also need to think of we need separate RPC handler
                     // threads for this?
                     ServerRpcController controller = new ServerRpcController();
+                    for (InvalidateServerMetadataCacheRequest invalidateCacheRequest:

Review Comment:
   > The list is very small so maybe it does not matter?
   
   Thats what I was thinking too. Atmost there will be 2 entries.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355867703


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3507,23 +3525,30 @@ private void invalidateServerMetadataCacheWithRetries(Admin admin,
                     // DDL operations. We also need to think of we need separate RPC handler
                     // threads for this?
                     ServerRpcController controller = new ServerRpcController();
+                    for (InvalidateServerMetadataCacheRequest invalidateCacheRequest:

Review Comment:
   It might still get rid of some code and bytes to string conversion. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1353969055


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -2417,6 +2417,15 @@ public void createTable(RpcController controller, CreateTableRequest request,
                 // table/index/views.
                 tableMetadata.add(MetaDataUtil.getLastDDLTimestampUpdate(tableKey,
                     clientTimeStamp, EnvironmentEdgeManager.currentTimeMillis()));
+                if (tableType == INDEX) {

Review Comment:
   Yeah, I realized that and withdrew the review ask. :P



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1353505435


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -2417,6 +2417,15 @@ public void createTable(RpcController controller, CreateTableRequest request,
                 // table/index/views.
                 tableMetadata.add(MetaDataUtil.getLastDDLTimestampUpdate(tableKey,
                     clientTimeStamp, EnvironmentEdgeManager.currentTimeMillis()));
+                if (tableType == INDEX) {

Review Comment:
   @shahrs87 We also need to do this when dropping an index, right? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#issuecomment-1758594036

   @palashc  The patch is ready to review now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7056 Update the LAST_DDL_TIMESTAMP of the parent table/view when we add/drop an index. [phoenix]

Posted by "shahrs87 (via GitHub)" <gi...@apache.org>.
shahrs87 commented on code in PR #1702:
URL: https://github.com/apache/phoenix/pull/1702#discussion_r1355870002


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3507,23 +3525,30 @@ private void invalidateServerMetadataCacheWithRetries(Admin admin,
                     // DDL operations. We also need to think of we need separate RPC handler
                     // threads for this?
                     ServerRpcController controller = new ServerRpcController();
+                    for (InvalidateServerMetadataCacheRequest invalidateCacheRequest:

Review Comment:
   But then it won't have the region server name. Having a regionserver name would be helpful.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org