Skip to content

Commit adcc3e5

Browse files
authored
[ML] Check invariants after restoration (elastic#1737)
Add checking of invariants after restoration of several classes in the model library. Also change a number of conditions where an error was emitted during the restoration process to be hard stops with LOG_ABORT.
1 parent a3bdc09 commit adcc3e5

5 files changed

+15
-9
lines changed

lib/model/CDetectorEqualizer.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ bool CDetectorEqualizer::acceptRestoreTraverser(core::CStateRestoreTraverser& tr
4545
/**/)
4646
if (name == SKETCH_TAG) {
4747
if (!detector) {
48-
LOG_ERROR(<< "Expected the detector label first");
49-
return false;
48+
LOG_ABORT(<< "Expected the detector label first");
5049
}
5150
m_Sketches.emplace_back(
5251
*detector, maths::CQuantileSketch(SKETCH_INTERPOLATION, SKETCH_SIZE));

lib/model/CEventRateBucketGatherer.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,9 @@ bool restoreAttributePeopleData(core::CStateRestoreTraverser& traverser, TSizeUS
231231
data.resize(lastCid + 1);
232232
}
233233
} else if (name == PERSON_TAG) {
234-
if (!seenCid) {
235-
LOG_ERROR(<< "Incorrect format - person ID before attribute ID in "
234+
if (seenCid == false) {
235+
LOG_ABORT(<< "Incorrect format - person ID before attribute ID in "
236236
<< traverser.value());
237-
return false;
238237
}
239238
std::size_t pid = 0;
240239
if (core::CStringUtils::stringToType(traverser.value(), pid) == false) {

lib/model/CIndividualModel.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,8 @@ bool CIndividualModel::doAcceptRestoreTraverser(core::CStateRestoreTraverser& tr
416416
}
417417
}
418418

419+
VIOLATES_INVARIANT(m_FirstBucketTimes.size(), !=, m_LastBucketTimes.size());
420+
419421
return true;
420422
}
421423

lib/model/CPopulationModel.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,9 @@ bool CPopulationModel::doAcceptRestoreTraverser(core::CStateRestoreTraverser& tr
323323
}
324324
} while (traverser.next());
325325

326+
VIOLATES_INVARIANT(m_AttributeFirstBucketTimes.size(), !=,
327+
m_AttributeLastBucketTimes.size());
328+
326329
return true;
327330
}
328331

lib/model/CTokenListCategory.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <core/CStatePersistInserter.h>
1111
#include <core/CStateRestoreTraverser.h>
1212
#include <core/CStringUtils.h>
13+
#include <core/RestoreMacros.h>
1314

1415
#include <functional>
1516

@@ -100,9 +101,8 @@ bool CTokenListCategory::acceptRestoreTraverser(core::CStateRestoreTraverser& tr
100101
m_BaseTokenIds.push_back(tokenAndWeight);
101102
} else if (name == BASE_TOKEN_WEIGHT) {
102103
if (m_BaseTokenIds.empty()) {
103-
LOG_ERROR(<< "Base token weight precedes base token ID in "
104+
LOG_ABORT(<< "Base token weight precedes base token ID in "
104105
<< traverser.value());
105-
return false;
106106
}
107107

108108
TSizeSizePr& tokenAndWeight = m_BaseTokenIds.back();
@@ -162,12 +162,12 @@ bool CTokenListCategory::acceptRestoreTraverser(core::CStateRestoreTraverser& tr
162162
} else if (name == ORIG_UNIQUE_TOKEN_WEIGHT) {
163163
if (core::CStringUtils::stringToType(traverser.value(),
164164
m_OrigUniqueTokenWeight) == false) {
165-
LOG_ERROR(<< "Invalid maximum string length in " << traverser.value());
165+
LOG_ERROR(<< "Invalid unique token weight in " << traverser.value());
166166
return false;
167167
}
168168
} else if (name == NUM_MATCHES) {
169169
if (core::CStringUtils::stringToType(traverser.value(), m_NumMatches) == false) {
170-
LOG_ERROR(<< "Invalid maximum string length in " << traverser.value());
170+
LOG_ERROR(<< "Invalid number of matches in " << traverser.value());
171171
return false;
172172
}
173173
} else if (name == BASE_RAW_STRING_LENGTH) {
@@ -178,6 +178,9 @@ bool CTokenListCategory::acceptRestoreTraverser(core::CStateRestoreTraverser& tr
178178
}
179179
} while (traverser.next());
180180

181+
// Ensure that m_CommonUniqueTokenIds is sorted in ascending order.
182+
std::sort(m_CommonUniqueTokenIds.begin(), m_CommonUniqueTokenIds.end(), CTokenIdLess{});
183+
181184
// m_BaseRawStringLen will only have been persisted by 7.9 and above.
182185
// In this case the absolute maximum set at the beginning of the method
183186
// will still be set. A reasonable compromise that will result in

0 commit comments

Comments
 (0)