Skip to content

Commit 7d8068f

Browse files
dwsteeledwsteele
committed
Don't decode manifest data when it is generated on a remote.
Decoding a manifest from the JSON provided by C to the hash required by Perl is an expensive process. If manifest() was called on a remote it was being decoded into a hash and then immediately re-encoded into JSON for transmission over the protocol layer. Instead, provide a function for the remote to get the raw JSON which can be transmitted as is and decoded in the calling process instead. This makes remote manifest calls as fast as they were before 2.16, but local calls must still pay the decoding penalty and are therefore slower. This will continue to be true until the Perl storage interface is retired at the end of the C migration. Note that for reasonable numbers of tables there is no detectable difference. The case in question involved 250K tables with a 10 minute decode time (which was being doubled) on a fast workstation.
1 parent 1e55b87 commit 7d8068f

File tree

4 files changed

+69
-6
lines changed

4 files changed

+69
-6
lines changed

‎lib/pgBackRest/Protocol/Remote/Minion.pm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ sub init
112112
&OP_STORAGE_CIPHER_PASS_USER => sub {$oStorage->cipherPassUser()},
113113
&OP_STORAGE_EXISTS => sub {$oStorage->exists(@{shift()})},
114114
&OP_STORAGE_LIST => sub {$oStorage->list(@{shift()})},
115-
&OP_STORAGE_MANIFEST => sub {$oStorage->manifest(@{shift()})},
115+
&OP_STORAGE_MANIFEST => sub {$oStorage->manifestJson(@{shift()})},
116116
&OP_STORAGE_MOVE => sub {$oStorage->move(@{shift()})},
117117
&OP_STORAGE_PATH_GET => sub {$oStorage->pathGet(@{shift()})},
118118
&OP_STORAGE_HASH_SIZE => sub {$oStorage->hashSize(@{shift()})},

‎lib/pgBackRest/Protocol/Storage/Remote.pm

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ use warnings FATAL => qw(all);
99
use Carp qw(confess);
1010
use English '-no_match_vars';
1111

12+
use JSON::PP;
13+
1214
use pgBackRest::Common::Exception;
1315
use pgBackRest::Common::Log;
1416
use pgBackRest::Config::Config;
@@ -42,6 +44,9 @@ sub new
4244
# Set variables
4345
$self->{oProtocol} = $oProtocol;
4446

47+
# Create JSON object
48+
$self->{oJSON} = JSON::PP->new()->allow_nonref();
49+
4550
# Return from function and log return values if any
4651
return logDebugReturn
4752
(
@@ -163,7 +168,7 @@ sub manifest
163168
{name => 'rhParam', required => false},
164169
);
165170

166-
my $hManifest = $self->{oProtocol}->cmdExecute(OP_STORAGE_MANIFEST, [$strPathExp, $rhParam]);
171+
my $hManifest = $self->{oJSON}->decode($self->{oProtocol}->cmdExecute(OP_STORAGE_MANIFEST, [$strPathExp, $rhParam]));
167172

168173
# Return from function and log return values if any
169174
return logDebugReturn

‎lib/pgBackRest/Storage/Storage.pm

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ sub manifest
400400
{name => 'strFilter', optional => true, trace => true},
401401
);
402402

403-
my $hManifest = $self->{oJSON}->decode($self->{oStorageC}->manifest($strPathExp, $strFilter));
403+
my $hManifest = $self->{oJSON}->decode($self->manifestJson($strPathExp, {strFilter => $strFilter}));
404404

405405
# Return from function and log return values if any
406406
return logDebugReturn
@@ -410,6 +410,34 @@ sub manifest
410410
);
411411
}
412412

413+
sub manifestJson
414+
{
415+
my $self = shift;
416+
417+
# Assign function parameters, defaults, and log debug info
418+
my
419+
(
420+
$strOperation,
421+
$strPathExp,
422+
$strFilter,
423+
) =
424+
logDebugParam
425+
(
426+
__PACKAGE__ . '->manifestJson', \@_,
427+
{name => 'strPathExp'},
428+
{name => 'strFilter', optional => true, trace => true},
429+
);
430+
431+
my $strManifestJson = $self->{oStorageC}->manifest($strPathExp, $strFilter);
432+
433+
# Return from function and log return values if any
434+
return logDebugReturn
435+
(
436+
$strOperation,
437+
{name => 'strManifestJson', value => $strManifestJson, trace => true}
438+
);
439+
}
440+
413441
####################################################################################################################################
414442
# move - move path/file
415443
####################################################################################################################################

‎src/perl/embed.auto.c

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11512,7 +11512,7 @@ static const EmbeddedModule embeddedModule[] =
1151211512
"&OP_STORAGE_CIPHER_PASS_USER => sub {$oStorage->cipherPassUser()},\n"
1151311513
"&OP_STORAGE_EXISTS => sub {$oStorage->exists(@{shift()})},\n"
1151411514
"&OP_STORAGE_LIST => sub {$oStorage->list(@{shift()})},\n"
11515-
"&OP_STORAGE_MANIFEST => sub {$oStorage->manifest(@{shift()})},\n"
11515+
"&OP_STORAGE_MANIFEST => sub {$oStorage->manifestJson(@{shift()})},\n"
1151611516
"&OP_STORAGE_MOVE => sub {$oStorage->move(@{shift()})},\n"
1151711517
"&OP_STORAGE_PATH_GET => sub {$oStorage->pathGet(@{shift()})},\n"
1151811518
"&OP_STORAGE_HASH_SIZE => sub {$oStorage->hashSize(@{shift()})},\n"
@@ -11811,6 +11811,8 @@ static const EmbeddedModule embeddedModule[] =
1181111811
"use Carp qw(confess);\n"
1181211812
"use English '-no_match_vars';\n"
1181311813
"\n"
11814+
"use JSON::PP;\n"
11815+
"\n"
1181411816
"use pgBackRest::Common::Exception;\n"
1181511817
"use pgBackRest::Common::Log;\n"
1181611818
"use pgBackRest::Config::Config;\n"
@@ -11838,6 +11840,8 @@ static const EmbeddedModule embeddedModule[] =
1183811840
"\n\n"
1183911841
"$self->{oProtocol} = $oProtocol;\n"
1184011842
"\n\n"
11843+
"$self->{oJSON} = JSON::PP->new()->allow_nonref();\n"
11844+
"\n\n"
1184111845
"return logDebugReturn\n"
1184211846
"(\n"
1184311847
"$strOperation,\n"
@@ -11939,7 +11943,7 @@ static const EmbeddedModule embeddedModule[] =
1193911943
"{name => 'rhParam', required => false},\n"
1194011944
");\n"
1194111945
"\n"
11942-
"my $hManifest = $self->{oProtocol}->cmdExecute(OP_STORAGE_MANIFEST, [$strPathExp, $rhParam]);\n"
11946+
"my $hManifest = $self->{oJSON}->decode($self->{oProtocol}->cmdExecute(OP_STORAGE_MANIFEST, [$strPathExp, $rhParam]));\n"
1194311947
"\n\n"
1194411948
"return logDebugReturn\n"
1194511949
"(\n"
@@ -14045,14 +14049,40 @@ static const EmbeddedModule embeddedModule[] =
1404514049
"{name => 'strFilter', optional => true, trace => true},\n"
1404614050
");\n"
1404714051
"\n"
14048-
"my $hManifest = $self->{oJSON}->decode($self->{oStorageC}->manifest($strPathExp, $strFilter));\n"
14052+
"my $hManifest = $self->{oJSON}->decode($self->manifestJson($strPathExp, {strFilter => $strFilter}));\n"
1404914053
"\n\n"
1405014054
"return logDebugReturn\n"
1405114055
"(\n"
1405214056
"$strOperation,\n"
1405314057
"{name => 'hManifest', value => $hManifest, trace => true}\n"
1405414058
");\n"
1405514059
"}\n"
14060+
"\n"
14061+
"sub manifestJson\n"
14062+
"{\n"
14063+
"my $self = shift;\n"
14064+
"\n\n"
14065+
"my\n"
14066+
"(\n"
14067+
"$strOperation,\n"
14068+
"$strPathExp,\n"
14069+
"$strFilter,\n"
14070+
") =\n"
14071+
"logDebugParam\n"
14072+
"(\n"
14073+
"__PACKAGE__ . '->manifestJson', \\@_,\n"
14074+
"{name => 'strPathExp'},\n"
14075+
"{name => 'strFilter', optional => true, trace => true},\n"
14076+
");\n"
14077+
"\n"
14078+
"my $strManifestJson = $self->{oStorageC}->manifest($strPathExp, $strFilter);\n"
14079+
"\n\n"
14080+
"return logDebugReturn\n"
14081+
"(\n"
14082+
"$strOperation,\n"
14083+
"{name => 'strManifestJson', value => $strManifestJson, trace => true}\n"
14084+
");\n"
14085+
"}\n"
1405614086
"\n\n\n\n"
1405714087
"sub move\n"
1405814088
"{\n"

0 commit comments

Comments
 (0)