haridsv commented on code in PR #2196:
URL: https://github.com/apache/phoenix/pull/2196#discussion_r2158092669
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3333,6 +3333,11 @@ private MetaDataMutationResult doDropTable(byte[] key,
byte[] tenantId, byte[] s
// Recursively delete indexes
for (byte[] indexName : indexNames) {
+ if (CDCUtil.isCDCIndex(indexName)) {
+ byte[] cdcKey = SchemaUtil.getTableKey(tenantId, schemaName,
CDCUtil.getCdcObjectName(indexName));
+ Delete deleteCdc = new Delete(cdcKey, clientTimeStamp);
+ catalogMutations.add(deleteCdc);
+ }
Review Comment:
The thought process there was that it is OK to go ahead and drop the CDC
object (main user intention), even if the index gets left behind. However, in
this case, the user intention is to drop the table and if for some reason
dropping index fails, it would have left the table in tact, but would have
dropped the CDC.
On the other hand, reordering the drop operations is only a slight
improvement, as a failure in dropping the table can already cause a partial
cleanup. Besides, since your logic is detecting the CDC via the index, if the
index is dropped first and a failure occurs before CDC object is dropped, then
a retry will not be able to discover it, so on a second thought, the current
order seems better. You may want to leave a quick comment on why we should
drop CDC first.
--
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-unsubscr...@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
