my-ship-it commented on code in PR #1656:
URL: https://github.com/apache/cloudberry/pull/1656#discussion_r3020648124
##########
src/backend/access/appendonly/aomd.c:
##########
@@ -294,6 +295,19 @@ mdunlink_ao(RelFileNodeBackend rnode, ForkNumber
forkNumber, bool isRedo)
pfree(segPath);
}
+ /*
+ * Delete or truncate the first segment. See mdunlinkfork also.
+ */
+ if (RelFileNodeBackendIsTemp(rnode))
+ {
+ /* Next unlink the file, unless it was already found to be
missing */
Review Comment:
Thanks for catching this！
Minor comments below:
int ret; is declared at function scope.
Better to be scoped inside the if block, or removed entirely since it's only
checked against < 0 immediately.
##########
src/backend/access/appendonly/aomd.c:
##########
@@ -294,6 +295,19 @@ mdunlink_ao(RelFileNodeBackend rnode, ForkNumber
forkNumber, bool isRedo)
pfree(segPath);
}
+ /*
+ * Delete or truncate the first segment. See mdunlinkfork also.
+ */
+ if (RelFileNodeBackendIsTemp(rnode))
+ {
+ /* Next unlink the file, unless it was already found to be
missing */
+ ret = unlink(path);
+ if (ret < 0 && errno != ENOENT)
+ ereport(WARNING,
+ (errcode_for_file_access(),
+ errmsg("could not remove file
\"%s\": %m", path)));
+ }
+
Review Comment:
Should also unlink the segment files beyond segment 0?
How about modifying mdunlink_ao_base_relfile() and mdunlink_ao_perFile() to
check RelFileNodeBackendIsTemp(rnode) and unlink immediately (mirroring the
heap mdunlinkfork pattern), rather than appending a separate unlink at the end
of mdunlink_ao?
Something like
```
if (!unlinkFiles->isRedo)
{
if (RelFileNodeBackendIsTemp(unlinkFiles->rnode))
{
ret = unlink(baserel);
if (ret < 0 && errno != ENOENT)
ereport(WARNING, ...);
}
else
{
// Existing truncate + deferred unlink logic
...
}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
