You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by "reshke (via GitHub)" <gi...@apache.org> on 2026/04/01 08:22:34 UTC
[PR] Always unlink first segment for AO/AOCO temp relation. [cloudberry]
reshke opened a new pull request, #1656:
URL: https://github.com/apache/cloudberry/pull/1656
https://github.com/apache/cloudberry/issues/1644
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Always unlink first segment for AO/AOCO temp relation. [cloudberry]
Posted by "my-ship-it (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Always unlink first segment for AO/AOCO temp relation. [cloudberry]
Posted by "andr-sokolov (via GitHub)" <gi...@apache.org>.
andr-sokolov commented on PR #1656:
URL: https://github.com/apache/cloudberry/pull/1656#issuecomment-4222336533
I suggest to add a test to check that the file is unlinked
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Always unlink first segment for AO/AOCO temp relation. [cloudberry]
Posted by "tuhaihe (via GitHub) via commits" <co...@cloudberry.apache.org>.
tuhaihe commented on PR #1656:
URL: https://github.com/apache/cloudberry/pull/1656#issuecomment-6814609856
I'm cleaning up the stale branches. If something is wrong, feel free to create a new branch in your own repository.
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Always unlink first segment for AO/AOCO temp relation. [cloudberry]
Posted by "tuhaihe (via GitHub) via commits" <co...@cloudberry.apache.org>.
tuhaihe closed pull request #1656: Always unlink first segment for AO/AOCO temp relation.
URL: https://github.com/apache/cloudberry/pull/1656
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org