Skip to content

Commit eac2bb6

Browse files
authored
Do not connect to the database if the replication configuration is incorrect (#8381)
* Do not connect to the database if the replication configuration is incorrect Configuration errors can lead to loss of synchronization between master and replica. * Send config errors to the client app only if report_errors is enabled * Report a replication error if it occurs during the initial connection to the replica and the report_errors is enabled * Add error "all configured replicas skipped"
1 parent 06aaf94 commit eac2bb6

File tree

2 files changed

+125
-28
lines changed

2 files changed

+125
-28
lines changed

src/jrd/replication/Config.cpp

Lines changed: 121 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ namespace
6262
constexpr ULONG DEFAULT_GROUP_FLUSH_DELAY = 0;
6363
constexpr ULONG DEFAULT_APPLY_IDLE_TIMEOUT = 10; // seconds
6464
constexpr ULONG DEFAULT_APPLY_ERROR_TIMEOUT = 60; // seconds
65+
constexpr bool DEFAULT_REPORT_ERRORS = false;
6566

6667
void parseLong(const string& input, ULONG& output)
6768
{
@@ -79,36 +80,53 @@ namespace
7980
output = false;
8081
}
8182

82-
void configError(const string& type, const string& key, const string& value)
83+
void configError(CheckStatusWrapper* status, const string& type, const string& key, const string& value)
8384
{
84-
raiseError("%s specifies %s: %s", key.c_str(), type.c_str(), value.c_str());
85+
string msg;
86+
if (!(status->getState() & IStatus::STATE_ERRORS))
87+
{
88+
msg.printf("Incorrect entry in %s", REPLICATION_CFGFILE);
89+
(Arg::Gds(isc_random) << Arg::Str(msg)).appendTo(status);
90+
}
91+
92+
msg.printf("%s specifies %s: %s", key.c_str(), type.c_str(), value.c_str());
93+
(Arg::Gds(isc_random) << Arg::Str(msg)).appendTo(status);
8594
}
8695

87-
void checkAccess(const PathName& path, const string& key)
96+
bool checkAccess(CheckStatusWrapper* status, const PathName& path, const string& key)
8897
{
8998
if (path.hasData() && !PathUtils::canAccess(path, 6))
90-
configError("missing or inaccessible directory", key, path.c_str());
99+
{
100+
configError(status, "missing or inaccessible directory", key, path.c_str());
101+
return false;
102+
}
103+
return true;
91104
}
92105

93106
void composeError(CheckStatusWrapper* status, const Exception& ex)
94107
{
95-
string prefix;
96-
prefix.printf("Incorrect entry in %s", REPLICATION_CFGFILE);
97-
98108
Arg::StatusVector sv;
99-
sv << Arg::Gds(isc_random) << Arg::Str(prefix);
109+
110+
if (!(status->getState() & IStatus::STATE_ERRORS))
111+
{
112+
string prefix;
113+
prefix.printf("Incorrect entry in %s", REPLICATION_CFGFILE);
114+
sv << Arg::Gds(isc_random) << Arg::Str(prefix);
115+
}
116+
117+
sv << Arg::StatusVector(status);
100118
sv << Arg::StatusVector(ex);
101119

102120
status->setErrors(sv.value());
103121
}
104122

105-
void parseExternalValue(const string& key, const string& value, string& output)
123+
bool parseExternalValue(CheckStatusWrapper* status, const string& key, const string& value, string& output)
106124
{
107125
const auto pos = key.rfind('_');
108126
if (pos == string::npos)
109127
{
110128
output = value.c_str();
111-
return;
129+
return true;
112130
}
113131

114132
string temp;
@@ -118,7 +136,10 @@ namespace
118136
{
119137
fb_utils::readenv(value.c_str(), temp);
120138
if (temp.isEmpty())
121-
configError("missing environment variable", key, value);
139+
{
140+
configError(status, "missing environment variable", key, value);
141+
return false;
142+
}
122143
}
123144
else if (key_source.equals(KEY_SUFFIX_FILE))
124145
{
@@ -129,44 +150,65 @@ namespace
129150

130151
AutoPtr<FILE> file(os_utils::fopen(filename.c_str(), "rt"));
131152
if (!file)
132-
configError("missing or inaccessible file", key, filename.c_str());
153+
{
154+
configError(status, "missing or inaccessible file", key, filename.c_str());
155+
return false;
156+
}
133157

134158
if (temp.LoadFromFile(file))
135159
temp.alltrim("\r");
136160

137161
if (temp.isEmpty())
138-
configError("first empty line of file", key, filename.c_str());
162+
{
163+
configError(status, "first empty line of file", key, filename.c_str());
164+
return false;
165+
}
139166
}
140167

141168
output = temp.c_str();
169+
return true;
142170
}
143171

144-
void parseSyncReplica(const ConfigFile::Parameters& params, SyncReplica& output)
172+
bool parseSyncReplica(CheckStatusWrapper* status, const ConfigFile::Parameters& params, SyncReplica& output)
145173
{
174+
bool result = true;
146175
for (const auto& el : params)
147176
{
148177
const string key(el.name.c_str());
149178
const string value(el.value);
150179

151180
if (value.isEmpty())
152-
continue;
181+
{
182+
configError(status, "empty value", output.database, key);
183+
result = false;
184+
}
153185

154186
if (key.find("username") == 0)
155187
{
156188
if (output.username.hasData())
157-
configError("multiple values", output.database, "username");
158-
parseExternalValue(key, value, output.username);
189+
{
190+
configError(status, "multiple values", output.database, "username");
191+
result = false;
192+
}
193+
result &= parseExternalValue(status, key, value, output.username);
159194
output.username.rtrim(" ");
160195
}
161196
else if (key.find("password") == 0)
162197
{
163198
if (output.password.hasData())
164-
configError("multiple values", output.database, "password");
165-
parseExternalValue(key, value, output.password);
199+
{
200+
configError(status, "multiple values", output.database, "password");
201+
result = false;
202+
}
203+
result &= parseExternalValue(status, key, value, output.password);
166204
}
167205
else
168-
configError("unknown parameter", output.database, key);
206+
{
207+
configError(status, "unknown key", output.database, key);
208+
result = false;
209+
}
169210
}
211+
return result;
170212
}
171213
}
172214

@@ -196,7 +238,7 @@ Config::Config()
196238
schemaSearchPath(getPool()),
197239
pluginName(getPool()),
198240
logErrors(true),
199-
reportErrors(false),
241+
reportErrors(DEFAULT_REPORT_ERRORS),
200242
disableOnError(true),
201243
cascadeReplication(false)
202244
{
@@ -238,6 +280,7 @@ Config* Config::get(const PathName& lookupName)
238280
{
239281
fb_assert(lookupName.hasData());
240282

283+
bool reportErrors = DEFAULT_REPORT_ERRORS;
241284
try
242285
{
243286
const PathName filename =
@@ -249,7 +292,8 @@ Config* Config::get(const PathName& lookupName)
249292

250293
AutoPtr<Config> config(FB_NEW Config);
251294

252-
bool defaultFound = false, exactMatch = false;
295+
FbLocalStatus localStatus;
296+
bool defaultFound = false, exactMatch = false, replicaSkip = false;
253297

254298
for (const auto& section : cfgFile.getParameters())
255299
{
@@ -287,15 +331,24 @@ Config* Config::get(const PathName& lookupName)
287331
string value(el.value);
288332

289333
if (value.isEmpty())
334+
{
335+
configError(&localStatus, "empty value of key",
336+
exactMatch ? lookupName.c_str() : section.name.c_str(),
337+
key);
290338
continue;
339+
}
291340

292341
if (key == "sync_replica")
293342
{
294343
SyncReplica syncReplica(config->getPool());
295344
if (el.sub)
296345
{
297346
syncReplica.database = value;
298-
parseSyncReplica(el.sub->getParameters(), syncReplica);
347+
if (!parseSyncReplica(&localStatus, el.sub->getParameters(), syncReplica))
348+
{
349+
replicaSkip = true;
350+
continue;
351+
}
299352
}
300353
else
301354
splitConnectionString(value, syncReplica.database, syncReplica.username, syncReplica.password);
@@ -338,7 +391,12 @@ Config* Config::get(const PathName& lookupName)
338391
{
339392
config->journalDirectory = value.c_str();
340393
PathUtils::ensureSeparator(config->journalDirectory);
341-
checkAccess(config->journalDirectory, key);
394+
if (!checkAccess(&localStatus, config->journalDirectory, key))
395+
{
396+
config->journalDirectory.erase();
397+
replicaSkip = true;
398+
continue;
399+
}
342400
}
343401
else if (key == "journal_file_prefix")
344402
{
@@ -352,7 +410,11 @@ Config* Config::get(const PathName& lookupName)
352410
{
353411
config->archiveDirectory = value.c_str();
354412
PathUtils::ensureSeparator(config->archiveDirectory);
355-
checkAccess(config->archiveDirectory, key);
413+
if (!checkAccess(&localStatus, config->archiveDirectory, key))
414+
{
415+
config->archiveDirectory.erase();
416+
continue;
417+
}
356418
}
357419
else if (key == "journal_archive_command")
358420
{
@@ -373,6 +435,7 @@ Config* Config::get(const PathName& lookupName)
373435
else if (key == "report_errors")
374436
{
375437
parseBoolean(value, config->reportErrors);
438+
reportErrors = config->reportErrors;
376439
}
377440
else if (key == "disable_on_error")
378441
{
@@ -382,12 +445,28 @@ Config* Config::get(const PathName& lookupName)
382445
{
383446
parseBoolean(value, config->cascadeReplication);
384447
}
448+
else if ((key != "journal_source_directory") &&
449+
(key != "source_guid") &&
450+
(key != "verbose_logging") &&
451+
(key != "apply_idle_timeout") &&
452+
(key != "apply_error_timeout"))
453+
{
454+
configError(&localStatus, "unknown key",
455+
exactMatch ? lookupName.c_str() : section.name.c_str(),
456+
key);
457+
}
385458
}
386459

387460
if (exactMatch)
388461
break;
389462
}
390463

464+
if (localStatus->getState() & IStatus::STATE_ERRORS)
465+
logPrimaryStatus(lookupName, &localStatus);
466+
467+
if (replicaSkip && !config->disableOnError)
468+
raiseError("One or more replicas configured with errors");
469+
391470
// TODO: As soon as plugin name is moved into RDB$PUBLICATIONS,
392471
// delay config parse until real replication start
393472
if (config->pluginName.hasData())
@@ -410,13 +489,18 @@ Config* Config::get(const PathName& lookupName)
410489

411490
return config.release();
412491
}
492+
493+
if (replicaSkip)
494+
raiseError("All configured replicas skipped");
413495
}
414496
catch (const Exception& ex)
415497
{
416498
FbLocalStatus localStatus;
417499
composeError(&localStatus, ex);
418500

419501
logPrimaryStatus(lookupName, &localStatus);
502+
if (reportErrors)
503+
localStatus.raise();
420504
}
421505

422506
return nullptr;
@@ -428,6 +512,7 @@ Config* Config::get(const PathName& lookupName)
428512
void Config::enumerate(ReplicaList& replicas)
429513
{
430514
PathName dbName;
515+
FbLocalStatus localStatus;
431516

432517
try
433518
{
@@ -480,12 +565,20 @@ void Config::enumerate(ReplicaList& replicas)
480565
{
481566
config->sourceDirectory = value.c_str();
482567
PathUtils::ensureSeparator(config->sourceDirectory);
568+
if (!checkAccess(&localStatus, config->sourceDirectory, key))
569+
{
570+
config->sourceDirectory.erase();
571+
continue;
572+
}
483573
}
484574
else if (key == "source_guid")
485575
{
486576
config->sourceGuid = Guid::fromString(value);
487577
if (!config->sourceGuid)
488-
configError("invalid (misformatted) value", key, value);
578+
{
579+
configError(&localStatus, "invalid (misformatted) value", key, value);
580+
continue;
581+
}
489582
}
490583
else if (key == "verbose_logging")
491584
{
@@ -517,11 +610,11 @@ void Config::enumerate(ReplicaList& replicas)
517610
}
518611
catch (const Exception& ex)
519612
{
520-
FbLocalStatus localStatus;
521613
composeError(&localStatus, ex);
614+
}
522615

616+
if (localStatus->getState() & IStatus::STATE_ERRORS)
523617
logReplicaStatus(dbName, &localStatus);
524-
}
525618
}
526619

527620
// This routine is used for split input connection string to parts

src/jrd/replication/Manager.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ Manager::Manager(const string& dbId,
184184
if (localStatus->getState() & IStatus::STATE_ERRORS)
185185
{
186186
logPrimaryStatus(m_config->dbName, &localStatus);
187+
if (m_config->reportErrors)
188+
(Arg::Gds(isc_repl_error) << Arg::StatusVector(&localStatus)).raise();
187189
continue;
188190
}
189191

@@ -192,6 +194,8 @@ Manager::Manager(const string& dbId,
192194
{
193195
logPrimaryStatus(m_config->dbName, &localStatus);
194196
attachment->detach(&localStatus);
197+
if (m_config->reportErrors)
198+
(Arg::Gds(isc_repl_error) << Arg::StatusVector(&localStatus)).raise();
195199
continue;
196200
}
197201

0 commit comments

Comments
 (0)