[ML] Refactor foreground persistence #1473
Conversation
Refactor foreground persistence to allow for using supplied descriptors when snapshotting model state. This paves the way for upgrading model state.
lib/api/CAnomalyJob.cc
Outdated
|
|
||
| core_t::TTime snapshotTimestamp(core::CTimeUtils::now()); | ||
| const std::string snapshotId(core::CStringUtils::typeToString(snapshotTimestamp)); | ||
| const std::string description{"Periodic background persist at " + snapshotId}; |
There was a problem hiding this comment.
| const std::string description{"Periodic background persist at " + snapshotId}; | |
| const std::string description{"Periodic background persist at " + core::CTimeUtils::toIso8601(snapshotTimestamp)}; |
|
|
||
| // Compare snapshot ID embedded in the state string with the supplied value. | ||
| const std::string expectedId{"job_model_state_" + | ||
| std::to_string(snapshotTimestamp) + "#1"}; |
There was a problem hiding this comment.
| std::to_string(snapshotTimestamp) + "#1"}; | |
| snapshotId + "#1"}; |
It's just coincidence that the snapshot ID is the snapshot timestamp in this case. The document ID is supposed to include the snapshot ID regardless of whether it's the same as the snapshot timestamp.
| job.finalise(); | ||
|
|
||
| ml::core_t::TTime snapshotTimestamp{1283524206}; | ||
| const std::string snapshotId{ml::core::CStringUtils::typeToString(snapshotTimestamp)}; |
There was a problem hiding this comment.
It's probably best to set the snapshot ID to something different to the snapshot timestamp, just to prove that the code being tested doesn't mix up the snapshot ID and the snapshot timestamp.
| const std::string snapshotId{ml::core::CStringUtils::typeToString(snapshotTimestamp)}; | |
| const std::string snapshotId{"my_special_snapshot"}; |
| std::string& snapshotIdOut, | ||
| std::size_t& numDocsOut) { | ||
| LOG_DEBUG(<< "Persist complete with description: " << modelSnapshotReport.s_Description); | ||
| description = modelSnapshotReport.s_Description; |
There was a problem hiding this comment.
This could also capture the snapshot timestamp so tests can assert that is as expected too.
| // Check that the snapshot description and Id reported by the "persist complete" | ||
| // handler match those supplied to the persist function | ||
| BOOST_REQUIRE_EQUAL(description, description_); | ||
| BOOST_REQUIRE_EQUAL(snapshotId, snapshotId_); |
There was a problem hiding this comment.
Also assert that the snapshot timestamp is as expected.
|
The CI failed because of the change of Gradle task name in the ES repo. To get it to work you need to merge latest master into the PR branch. |
Refactor foreground persistence to allow for using supplied descriptors when snapshotting model state. This paves the way for upgrading model state.
2 participants
Refactor foreground persistence to allow for using supplied descriptors when snapshotting model state.
This paves the way for upgrading model state.