db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-10-30 23:37:45 +02:00
Code Issues Releases Activity

Add storagePathRemove() and use it in the Perl Posix driver.

Browse Source 
This implementation should be faster because it does not stat each file.  It simply assumes that most directory entries are files so attempts an unlink() first.  If the entry is reported by error codes to be a directory then it attempts an rmdir().
This commit is contained in:
David Steele
2018-04-11 08:21:09 -04:00
parent c9ce20d41a
commit 4744eb9387
13 changed files with 225 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
doc/xml/release.xml
View File

@@ -36,7 +36,7 @@
</release-item>
<release-item>
<p>Storage object improvements. Convert all functions to variadic functions. Enforce read-only storage. Add <code>storageLocalWrite()</code> helper function. Add <code>storageExists()</code>, <code>storagePathCreate()</code>, and <code>storageRemove()</code>. Add <code>StorageFile</code> object and <code>storageOpenRead()</code>/<code>storageOpenWrite()</code>. Abstract Posix driver code into a separate module.</p>
<p>Storage object improvements. Convert all functions to variadic functions. Enforce read-only storage. Add <code>storageLocalWrite()</code> helper function. Add <code>storageExists()</code>, <code>storagePathCreate()</code>, and <code>storageRemove()</code>. Add <code>StorageFile</code> object and <code>storageOpenRead()</code>/<code>storageOpenWrite()</code>. Abstract Posix driver code into a separate module. Add <code>storagePathRemove()</code> and use it in the Perl Posix driver.</p>
</release-item>
<release-item>

34
lib/pgBackRest/Storage/Posix/Driver.pm
View File

@@ -927,14 +927,14 @@ sub remove
my
(
$strOperation,
$xstryPathFile,
$strPathFile,
$bIgnoreMissing,
$bRecurse,
) =
logDebugParam
(
__PACKAGE__ . '->remove', \@_,
{name => 'xstryPathFile', trace => true},
{name => 'strPathFile', trace => true},
{name => 'bIgnoreMissing', optional => true, default => false, trace => true},
{name => 'bRecurse', optional => true, default => false, trace => true},
);
@@ -945,36 +945,16 @@ sub remove
# Remove a tree
if ($bRecurse)
{
my $oManifest = $self->manifest($xstryPathFile, {bIgnoreMissing => true});
# Dynamically load the driver
require pgBackRest::LibC;
pgBackRest::LibC->import(qw(:storage));
# Iterate all files in the manifest
foreach my $strFile (sort({$b cmp $a} keys(%{$oManifest})))
{
# remove directory
if ($oManifest->{$strFile}{type} eq 'd')
{
my $xstryPathFileRemove = $strFile eq '.' ? $xstryPathFile : "${xstryPathFile}/${strFile}";
if (!rmdir($xstryPathFileRemove))
{
# Throw error if this is not an ignored missing path
if (!($OS_ERROR{ENOENT} && $bIgnoreMissing))
{
logErrorResult(ERROR_PATH_REMOVE, "unable to remove path '${strFile}'", $OS_ERROR);
}
}
}
# Remove file
else
{
$self->remove("${xstryPathFile}/${strFile}", {bIgnoreMissing => true});
}
}
storageDriverPosixPathRemove($strPathFile, !$bIgnoreMissing, $bRecurse)
}
# Only remove the specified file
else
{
foreach my $strFile (ref($xstryPathFile) ? @{$xstryPathFile} : ($xstryPathFile))
foreach my $strFile (ref($strPathFile) ? @{$strPathFile} : ($strPathFile))
{
if (unlink($strFile) != 1)
{

40
libc/LibC.h
View File

@@ -65,9 +65,12 @@ Error handling macros that throw a Perl error when a C error is caught
/***********************************************************************************************************************************
Core context handling macros, only intended to be called from other macros
***********************************************************************************************************************************/
#define MEM_CONTEXT_XS_OLD() \
MEM_CONTEXT_XS_memContextOld
#define MEM_CONTEXT_XS_CORE_BEGIN(memContext) \
/* Switch to the new memory context */ \
MemContext *MEM_CONTEXT_XS_memContextOld = memContextSwitch(memContext); \
MemContext *MEM_CONTEXT_XS_OLD() = memContextSwitch(memContext); \
\
/* Store any errors to be croaked to Perl at the end */ \
bool MEM_CONTEXT_XS_croak = false; \
@@ -84,7 +87,7 @@ Core context handling macros, only intended to be called from other macros
/* Free the context on error */ \
FINALLY() \
{ \
memContextSwitch(MEM_CONTEXT_XS_memContextOld); \
memContextSwitch(MEM_CONTEXT_XS_OLD()); \
} \
TRY_END();
@@ -140,6 +143,39 @@ Simplifies switching the memory context in functions and includes error handling
} \
}
/***********************************************************************************************************************************
Simplifies switching to a temp memory context in functions and includes error handling
***********************************************************************************************************************************/
#define MEM_CONTEXT_XS_TEMP() \
MEM_CONTEXT_XS_TEMP_memContext
#define MEM_CONTEXT_XS_TEMP_BEGIN() \
{ \
MemContext *MEM_CONTEXT_XS_TEMP() = memContextNew("temporary"); \
\
MEM_CONTEXT_XS_CORE_BEGIN(MEM_CONTEXT_XS_TEMP())
#define MEM_CONTEXT_XS_TEMP_END() \
/* Set error to be croak to Perl later */ \
CATCH_ANY() \
{ \
MEM_CONTEXT_XS_croak = true; \
} \
/* Free the context on error */ \
FINALLY() \
{ \
memContextSwitch(MEM_CONTEXT_XS_OLD()); \
memContextFree(MEM_CONTEXT_XS_TEMP()); \
} \
TRY_END(); \
\
/* Croak on error */ \
if (MEM_CONTEXT_XS_croak) \
{ \
ERROR_XS() \
} \
}
/***********************************************************************************************************************************
Free memory context in destructors
***********************************************************************************************************************************/

2
libc/LibC.xs
View File

@@ -48,6 +48,7 @@ These includes are from the src directory. There is no Perl-specific code in th
#include "config/load.h"
#include "perl/config.h"
#include "postgres/pageChecksum.h"
#include "storage/driver/posix.h"
/***********************************************************************************************************************************
Helper macros
@@ -102,3 +103,4 @@ INCLUDE: xs/config/config.xs
INCLUDE: xs/config/configTest.xs
INCLUDE: xs/config/define.xs
INCLUDE: xs/postgres/pageChecksum.xs
INCLUDE: xs/storage/storage.xs

7
libc/build/lib/pgBackRestLibC/Build.pm
View File

@@ -115,6 +115,13 @@ my $rhExport =
)],
},
'storage' =>
{
&BLD_EXPORTTYPE_SUB => [qw(
storageDriverPosixPathRemove
)],
},
'test' =>
{
&BLD_EXPORTTYPE_SUB => [qw(

5
libc/lib/pgBackRest/LibCAuto.pm
View File

@@ -240,6 +240,11 @@ sub libcAutoExportTag
'randomBytes',
],
storage =>
[
'storageDriverPosixPathRemove',
],
test =>
[
'cfgParseTest',

18
libc/xs/storage/storage.xs Normal file
View File

@@ -0,0 +1,18 @@
# ----------------------------------------------------------------------------------------------------------------------------------
# Storage Exports
# ----------------------------------------------------------------------------------------------------------------------------------
MODULE = pgBackRest::LibC PACKAGE = pgBackRest::LibC
####################################################################################################################################
void
storageDriverPosixPathRemove(path, errorOnMissing, recurse)
const char *path
bool errorOnMissing
bool recurse
CODE:
MEM_CONTEXT_XS_TEMP_BEGIN()
{
storageDriverPosixPathRemove(strNew(path), errorOnMissing, recurse);
}
MEM_CONTEXT_XS_TEMP_END();

48
src/storage/driver/posix.c
View File

@@ -121,7 +121,7 @@ storageDriverPosixList(const String *path, bool errorOnMissing, const String *ex
if (!dir)
{
if (errorOnMissing || errno != ENOENT)
THROW_SYS_ERROR(PathOpenError, "unable to open directory '%s' for read", strPtr(path));
THROW_SYS_ERROR(PathOpenError, "unable to open path '%s' for read", strPtr(path));
}
else
{
@@ -244,6 +244,52 @@ storageDriverPosixPathCreate(const String *path, bool errorOnExists, bool noPare
}
}
/***********************************************************************************************************************************
Remove a path
***********************************************************************************************************************************/
void
storageDriverPosixPathRemove(const String *path, bool errorOnMissing, bool recurse)
{
MEM_CONTEXT_TEMP_BEGIN()
{
// Recurse if requested
if (recurse)
{
// Get a list of files in this path
StringList *fileList = storageDriverPosixList(path, errorOnMissing, NULL);
// Only continue if the path exists
if (fileList != NULL)
{
// Delete all paths and files
for (unsigned int fileIdx = 0; fileIdx < strLstSize(fileList); fileIdx++)
{
String *file = strNewFmt("%s/%s", strPtr(path), strPtr(strLstGet(fileList, fileIdx)));
// Rather than stat the file to discover what type it is, just try to unlink it and see what happens
if (unlink(strPtr(file)) == -1)
{
// These errors indicate that the entry is actually a path so we'll try to delete it that way
if (errno == EPERM || errno == EISDIR) // {uncovered - EPERM is not returned on tested systems}
storageDriverPosixPathRemove(file, false, true);
// Else error
else
THROW_SYS_ERROR(PathRemoveError, "unable to remove path/file '%s'", strPtr(file));
}
}
}
}
// Delete the path
if (rmdir(strPtr(path)) == -1)
{
if (errorOnMissing || errno != ENOENT)
THROW_SYS_ERROR(PathRemoveError, "unable to remove path '%s'", strPtr(path));
}
}
MEM_CONTEXT_TEMP_END();
}
/***********************************************************************************************************************************
Write a buffer to storage
***********************************************************************************************************************************/

1
src/storage/driver/posix.h
View File

@@ -20,6 +20,7 @@ StringList *storageDriverPosixList(const String *path, bool errorOnMissing, cons
void *storageDriverPosixOpenRead(const String *file, bool ignoreMissing);
void *storageDriverPosixOpenWrite(const String *file, mode_t mode);
void storageDriverPosixPathCreate(const String *path, bool errorOnExists, bool noParentCreate, mode_t mode);
void storageDriverPosixPathRemove(const String *path, bool errorOnMissing, bool recurse);
void storageDriverPosixPut(const StorageFile *file, const Buffer *buffer);
void storageDriverPosixRemove(const String *file, bool errorOnMissing);

19
src/storage/storage.c
View File

@@ -304,6 +304,25 @@ storagePathCreate(const Storage *this, const String *pathExp, StoragePathCreateP
MEM_CONTEXT_TEMP_END();
}
/***********************************************************************************************************************************
Remove a path
***********************************************************************************************************************************/
void
storagePathRemove(const Storage *this, const String *pathExp, StoragePathRemoveParam param)
{
ASSERT_STORAGE_ALLOWS_WRITE();
MEM_CONTEXT_TEMP_BEGIN()
{
// Build the path
String *path = storagePathNP(this, pathExp);
// Call driver function
storageDriverPosixPathRemove(path, param.errorOnMissing, param.recurse);
}
MEM_CONTEXT_TEMP_END();
}
/***********************************************************************************************************************************
Write a buffer to storage
***********************************************************************************************************************************/

16
src/storage/storage.h
View File

@@ -148,6 +148,22 @@ typedef struct StoragePathCreateParam
void storagePathCreate(const Storage *this, const String *pathExp, StoragePathCreateParam param);
/***********************************************************************************************************************************
storagePathRemove
***********************************************************************************************************************************/
typedef struct StoragePathRemoveParam
{
bool errorOnMissing;
bool recurse;
} StoragePathRemoveParam;
#define storagePathRemoveP(this, pathExp, ...) \
storagePathRemove(this, pathExp, (StoragePathRemoveParam){__VA_ARGS__})
#define storagePathRemoveNP(this, pathExp) \
storagePathRemove(this, pathExp, (StoragePathRemoveParam){0})
void storagePathRemove(const Storage *this, const String *pathExp, StoragePathRemoveParam param);
/***********************************************************************************************************************************
storagePut
***********************************************************************************************************************************/

2
test/define.yaml
View File

@@ -457,7 +457,7 @@ module:
# ----------------------------------------------------------------------------------------------------------------------------
- name: storage
total: 9
total: 10
c: true
coverage:

66
test/src/module/storage/storageTest.c
View File

@@ -123,19 +123,19 @@ testRun()
// -------------------------------------------------------------------------------------------------------------------------
TEST_ERROR(
storageListP(storageTest, strNew(BOGUS_STR), .errorOnMissing = true), PathOpenError,
strPtr(strNewFmt("unable to open directory '%s/BOGUS' for read: [2] No such file or directory", testPath())));
strPtr(strNewFmt("unable to open path '%s/BOGUS' for read: [2] No such file or directory", testPath())));
TEST_RESULT_PTR(storageListNP(storageTest, strNew(BOGUS_STR)), NULL, "ignore missing dir");
// -------------------------------------------------------------------------------------------------------------------------
TEST_ERROR(
storageListNP(storageTest, pathNoPerm), PathOpenError,
strPtr(strNewFmt("unable to open directory '%s' for read: [13] Permission denied", strPtr(pathNoPerm))));
strPtr(strNewFmt("unable to open path '%s' for read: [13] Permission denied", strPtr(pathNoPerm))));
// Should still error even when ignore missing
TEST_ERROR(
storageListNP(storageTest, pathNoPerm), PathOpenError,
strPtr(strNewFmt("unable to open directory '%s' for read: [13] Permission denied", strPtr(pathNoPerm))));
strPtr(strNewFmt("unable to open path '%s' for read: [13] Permission denied", strPtr(pathNoPerm))));
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_VOID(
@@ -215,6 +215,66 @@ testRun()
TEST_RESULT_INT(system(strPtr(strNewFmt("rm -rf %s/sub*", testPath()))), 0, "remove sub paths");
}
// *****************************************************************************************************************************
if (testBegin("storagePathRemove()"))
{
// -------------------------------------------------------------------------------------------------------------------------
String *pathRemove1 = strNewFmt("%s/remove1", testPath());
TEST_ERROR(
storagePathRemoveP(storageTest, pathRemove1, .errorOnMissing = true), PathRemoveError,
strPtr(strNewFmt("unable to remove path '%s': [2] No such file or directory", strPtr(pathRemove1))));
TEST_RESULT_VOID(storagePathRemoveP(storageTest, pathRemove1, .recurse = true), "ignore missing path");
// -------------------------------------------------------------------------------------------------------------------------
String *pathRemove2 = strNewFmt("%s/remove2", strPtr(pathRemove1));
TEST_RESULT_INT(system(strPtr(strNewFmt("sudo mkdir -p -m 700 %s", strPtr(pathRemove2)))), 0, "create noperm paths");
TEST_ERROR(
storagePathRemoveNP(storageTest, pathRemove2), PathRemoveError,
strPtr(strNewFmt("unable to remove path '%s': [13] Permission denied", strPtr(pathRemove2))));
TEST_ERROR(
storagePathRemoveP(storageTest, pathRemove2, .recurse = true), PathOpenError,
strPtr(strNewFmt("unable to open path '%s' for read: [13] Permission denied", strPtr(pathRemove2))));
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_INT(system(strPtr(strNewFmt("sudo chmod 777 %s", strPtr(pathRemove1)))), 0, "top path can be removed");
TEST_ERROR(
storagePathRemoveP(storageTest, pathRemove2, .recurse = true), PathOpenError,
strPtr(strNewFmt("unable to open path '%s' for read: [13] Permission denied", strPtr(pathRemove2))));
// -------------------------------------------------------------------------------------------------------------------------
String *fileRemove = strNewFmt("%s/remove.txt", strPtr(pathRemove2));
TEST_RESULT_INT(
system(strPtr(strNewFmt(
"sudo chmod 755 %s && sudo touch %s && sudo chmod 777 %s", strPtr(pathRemove2), strPtr(fileRemove),
strPtr(fileRemove)))),
0, "add no perm file");
TEST_ERROR(
storagePathRemoveP(storageTest, pathRemove1, .recurse = true), PathRemoveError,
strPtr(strNewFmt("unable to remove path/file '%s': [13] Permission denied", strPtr(fileRemove))));
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_INT(system(strPtr(strNewFmt("sudo chmod 777 %s", strPtr(pathRemove2)))), 0, "bottom path can be removed");
TEST_RESULT_VOID(
storagePathRemoveP(storageTest, pathRemove1, .recurse = true), "remove path");
TEST_RESULT_BOOL(
storageExistsNP(storageTest, pathRemove1), false, "path is removed");
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_INT(system(strPtr(strNewFmt("mkdir -p %s", strPtr(pathRemove2)))), 0, "create subpaths");
TEST_RESULT_VOID(
storagePathRemoveP(storageTest, pathRemove1, .recurse = true), "remove path");
TEST_RESULT_BOOL(
storageExistsNP(storageTest, pathRemove1), false, "path is removed");
}
// *****************************************************************************************************************************
if (testBegin("storageOpenRead()"))
{