From c93befb202264081ab14a5ce2db70e5c432fa909 Mon Sep 17 00:00:00 2001 From: VladiMihaylenko Date: Wed, 1 Aug 2018 12:48:57 +0300 Subject: [PATCH] [ugc] Fixed migration issues --- ugc/index_migration/utility.cpp | 21 +++-- ugc/index_migration/utility.hpp | 3 +- ugc/storage.cpp | 69 +++++++++++---- ugc/storage.hpp | 3 +- ugc/ugc_tests/CMakeLists.txt | 1 + ugc/ugc_tests/storage_tests.cpp | 152 ++++++++++++++++++++++++++++---- 6 files changed, 202 insertions(+), 47 deletions(-) diff --git a/ugc/index_migration/utility.cpp b/ugc/index_migration/utility.cpp index e97f9d8c1e..0275c0a38a 100644 --- a/ugc/index_migration/utility.cpp +++ b/ugc/index_migration/utility.cpp @@ -49,9 +49,13 @@ bool GetMigrationTable(int64_t tableVersion, MigrationTable & t) return true; } -bool MigrateFromV0ToV1(ugc::UpdateIndexes & source) +ugc::migration::Result MigrateFromV0ToV1(ugc::UpdateIndexes & source) { + using ugc::migration::Result; + MigrationTables tables; + + bool needDefragmentation = false; for (auto & index : source) { auto const version = index.m_dataVersion; @@ -59,7 +63,12 @@ bool MigrateFromV0ToV1(ugc::UpdateIndexes & source) { MigrationTable t; if (!GetMigrationTable(version, t)) - return false; + { + LOG(LINFO, ("Can't find migration table for version", version)); + index.m_deleted = true; + needDefragmentation = true; + continue; + } tables.emplace(version, move(t)); } @@ -71,7 +80,7 @@ bool MigrateFromV0ToV1(ugc::UpdateIndexes & source) index.m_version = ugc::IndexVersion::Latest; } - return true; + return needDefragmentation ? Result::NeedDefragmentation : Result::Success; } } // namespace @@ -82,11 +91,7 @@ namespace migration Result Migrate(UpdateIndexes & source) { CHECK(!source.empty(), ()); - if (source.front().m_version == IndexVersion::Latest) - return Result::UpToDate; - - auto const result = MigrateFromV0ToV1(source); - return result ? Result::Success : Result::Failure; + return MigrateFromV0ToV1(source); } } // namespace migration } // namespace ugc diff --git a/ugc/index_migration/utility.hpp b/ugc/index_migration/utility.hpp index 658ba3629f..68e8426a51 100644 --- a/ugc/index_migration/utility.hpp +++ b/ugc/index_migration/utility.hpp @@ -8,8 +8,7 @@ namespace migration { enum class Result { - UpToDate, - Failure, + NeedDefragmentation, Success }; diff --git a/ugc/storage.cpp b/ugc/storage.cpp index 6d9317a0d6..19d96e65aa 100644 --- a/ugc/storage.cpp +++ b/ugc/storage.cpp @@ -229,39 +229,64 @@ void Storage::Load() boost::optional version; for (auto const & i : m_indexes) { + if (i.m_deleted) + { + ++m_numberOfDeleted; + continue; + } + if (!version) version = i.m_version; else - CHECK_EQUAL(static_cast(*version), static_cast(i.m_version), ("Inconsistent index")); - - if (i.m_deleted) - ++m_numberOfDeleted; + CHECK_EQUAL(static_cast(*version), static_cast(i.m_version), ("Inconsistent index:", data)); } - Migrate(indexFilePath); + if (version && *version != IndexVersion::Latest) + Migrate(indexFilePath); } void Storage::Migrate(string const & indexFilePath) { - if (m_indexes.empty()) + CHECK(!m_indexes.empty(), ()); + + string const suffix = ".v0"; + auto const ugcFilePath = GetUGCFilePath(); + + // Backup existing files + auto const v0IndexFilePath = indexFilePath + suffix; + auto const v0UGCFilePath = ugcFilePath + suffix; + if (!my::CopyFileX(indexFilePath, v0IndexFilePath)) + { + LOG(LERROR, ("Can't backup UGC index file")); return; + } + + if (!my::CopyFileX(ugcFilePath, v0UGCFilePath)) + { + my::DeleteFileX(v0IndexFilePath); + LOG(LERROR, ("Can't backup UGC update file")); + return; + } switch (migration::Migrate(m_indexes)) { - case migration::Result::UpToDate: - break; - case migration::Result::Failure: - LOG(LWARNING, ("Index migration failed")); - break; + case migration::Result::NeedDefragmentation: + LOG(LINFO, ("Need defragmentation after successful UGC index migration")); + DefragmentationImpl(true /* force */); + // fallthrough case migration::Result::Success: - LOG(LINFO, ("Index migration successful")); - auto const newPath = indexFilePath + ".v0"; - my::RenameFileX(indexFilePath, newPath); if (!SaveIndex(indexFilePath)) { - my::RenameFileX(newPath, indexFilePath); - LOG(LWARNING, ("Saving index file after indexes migration failed")); + my::DeleteFileX(indexFilePath); + my::DeleteFileX(ugcFilePath); + my::RenameFileX(v0UGCFilePath, ugcFilePath); + my::RenameFileX(v0IndexFilePath, indexFilePath); + m_indexes.clear(); + LOG(LERROR, ("Can't save UGC index after migration")); + return; } + + LOG(LINFO, ("UGC index migration successful")); break; } } @@ -281,6 +306,7 @@ bool Storage::SaveIndex(std::string const & pathToTargetFile /* = "" */) const catch (FileWriter::Exception const & exception) { LOG(LERROR, ("Exception while writing file:", indexFilePath, "reason:", exception.what())); + my::DeleteFileX(indexFilePath); return false; } @@ -288,9 +314,14 @@ bool Storage::SaveIndex(std::string const & pathToTargetFile /* = "" */) const } void Storage::Defragmentation() +{ + DefragmentationImpl(false /* force */); +} + +void Storage::DefragmentationImpl(bool force) { auto const indexesSize = m_indexes.size(); - if (m_numberOfDeleted < indexesSize / 2) + if (!force && m_numberOfDeleted < indexesSize / 2) return; auto const ugcFilePath = GetUGCFilePath(); @@ -486,7 +517,9 @@ void Storage::LoadForTesting(std::string const & testIndexFilePath) CHECK(!data.empty(), ()); DeserializeIndexes(data, m_indexes); - Migrate(testIndexFilePath); + + if (m_indexes.front().m_version != IndexVersion::Latest) + Migrate(testIndexFilePath); } } // namespace ugc diff --git a/ugc/storage.hpp b/ugc/storage.hpp index 30ce16f8a4..b9282614e2 100644 --- a/ugc/storage.hpp +++ b/ugc/storage.hpp @@ -36,12 +36,13 @@ public: size_t GetNumberOfUnsynchronized() const; /// Testing - UpdateIndexes const & GetIndexesForTesting() const { return m_indexes; } + UpdateIndexes & GetIndexesForTesting() { return m_indexes; } size_t GetNumberOfDeletedForTesting() const { return m_numberOfDeleted; } SettingResult SetUGCUpdateForTesting(FeatureID const & id, v0::UGCUpdate const & ugc); void LoadForTesting(std::string const & testIndexFilePath); private: + void DefragmentationImpl(bool force); uint64_t UGCSizeAtIndex(size_t const indexPosition) const; std::unique_ptr GetFeature(FeatureID const & id) const; void Migrate(std::string const & indexFilePath); diff --git a/ugc/ugc_tests/CMakeLists.txt b/ugc/ugc_tests/CMakeLists.txt index 7b62970218..e1f036d136 100644 --- a/ugc/ugc_tests/CMakeLists.txt +++ b/ugc/ugc_tests/CMakeLists.txt @@ -37,6 +37,7 @@ omim_link_libraries( indexer storage platform + platform_tests_support coding geometry base diff --git a/ugc/ugc_tests/storage_tests.cpp b/ugc/ugc_tests/storage_tests.cpp index 6e848bde5f..3efd701dd8 100644 --- a/ugc/ugc_tests/storage_tests.cpp +++ b/ugc/ugc_tests/storage_tests.cpp @@ -24,6 +24,7 @@ #include "platform/local_country_file_utils.hpp" #include "platform/platform.hpp" +#include "platform/platform_tests_support/scoped_file.hpp" #include #include @@ -39,16 +40,47 @@ using namespace ugc; namespace { +int64_t constexpr kMinVersionForMigration = 171020; + string const kTestMwmName = "ugc storage test"; -bool DeleteIndexFile() +bool DeleteIndexFile(ugc::IndexVersion v = ugc::IndexVersion::Latest) { - return my::DeleteFileX(my::JoinPath(GetPlatform().WritableDir(), "index.json")); + if (v == ugc::IndexVersion::Latest) + return my::DeleteFileX(my::JoinPath(GetPlatform().WritableDir(), "index.json")); + + string version; + switch (v) + { + case ugc::IndexVersion::V0: + version = "v0"; + break; + case ugc::IndexVersion::V1: + version = "v1"; + break; + } + + return my::DeleteFileX(my::JoinPath(GetPlatform().WritableDir(), "index.json." + version)); } -bool DeleteUGCFile() +bool DeleteUGCFile(ugc::IndexVersion v = ugc::IndexVersion::Latest) { - return my::DeleteFileX(my::JoinPath(GetPlatform().WritableDir(), "ugc.update.bin")); + if (v == ugc::IndexVersion::Latest) + return my::DeleteFileX(my::JoinPath(GetPlatform().WritableDir(), "ugc.update.bin")); + + + string version; + switch (v) + { + case ugc::IndexVersion::V0: + version = "v0"; + break; + case ugc::IndexVersion::V1: + version = "v1"; + break; + } + + return my::DeleteFileX(my::JoinPath(GetPlatform().WritableDir(), "ugc.update.bin." + version)); } } // namespace @@ -120,7 +152,7 @@ private: m_testMwm = platform::LocalCountryFile(GetPlatform().WritableDir(), platform::CountryFile(kTestMwmName), - 0 /* version */); + kMinVersionForMigration); { generator::tests_support::TestMwmBuilder builder(m_testMwm, feature::DataHeader::country); fn(builder); @@ -438,6 +470,9 @@ UNIT_CLASS_TEST(StorageTest, GetNumberOfUnsentSeparately) UNIT_TEST(UGC_IndexMigrationFromV0ToV1Smoke) { + platform::tests_support::ScopedFile dummyUgcUpdate("ugc.update.bin", "some test content"); + LOG(LINFO, ("Created dummy ugc update", dummyUgcUpdate)); + auto & p = GetPlatform(); auto const version = "v0"; auto const indexFileName = "index.json"; @@ -457,21 +492,37 @@ UNIT_TEST(UGC_IndexMigrationFromV0ToV1Smoke) auto & builder = MwmBuilder::Builder(); builder.Build({}); - auto const v0IndexFilePath = indexFilePath + "." + version; - Storage s(builder.GetDataSource()); - s.LoadForTesting(indexFilePath); - uint64_t migratedIndexFileSize = 0; - uint64_t v0IndexFileSize = 0; - TEST(my::GetFileSize(indexFilePath, migratedIndexFileSize), ()); - TEST(my::GetFileSize(v0IndexFilePath, v0IndexFileSize), ()); - TEST_GREATER(migratedIndexFileSize, 0, ()); - TEST_GREATER(v0IndexFileSize, 0, ()); - auto const & indexes = s.GetIndexesForTesting(); - for (auto const & i : indexes) + { - TEST_EQUAL(static_cast(i.m_version), static_cast(IndexVersion::Latest), ()); - TEST(!i.m_synchronized, ()); + Storage s(builder.GetDataSource()); + s.LoadForTesting(indexFilePath); + uint64_t migratedIndexFileSize = 0; + uint64_t v0IndexFileSize = 0; + TEST(my::GetFileSize(indexFilePath, migratedIndexFileSize), ()); + TEST(my::GetFileSize(v0IndexFilePath, v0IndexFileSize), ()); + TEST_GREATER(migratedIndexFileSize, 0, ()); + TEST_GREATER(v0IndexFileSize, 0, ()); + auto const & indexes = s.GetIndexesForTesting(); + for (auto const & i : indexes) + { + TEST_EQUAL(static_cast(i.m_version), static_cast(IndexVersion::Latest), ()); + TEST(!i.m_synchronized, ()); + } + + TEST(s.SaveIndex(indexFilePath), ()); + } + + { + Storage s(builder.GetDataSource()); + s.LoadForTesting(indexFilePath); + auto const & indexes = s.GetIndexesForTesting(); + TEST_NOT_EQUAL(indexes.size(), 0, ()); + for (auto const & i : indexes) + { + TEST_EQUAL(static_cast(i.m_version), static_cast(IndexVersion::Latest), ()); + TEST(!i.m_synchronized, ()); + } } my::DeleteFileX(indexFilePath); @@ -488,3 +539,68 @@ UNIT_TEST(UGC_NoReviews) TEST(!DeleteIndexFile(), ()); TEST(!DeleteUGCFile(), ()); } + +UNIT_TEST(UGC_TooOldDataVersionsForMigration) +{ + auto & builder = MwmBuilder::Builder(); + m2::PointD const cafePoint(1.0, 1.0); + m2::PointD const railwayPoint(2.0, 2.0); + builder.Build({TestCafe(cafePoint), TestRailway(railwayPoint)}); + auto const cafeId = builder.FeatureIdForCafeAtPoint(cafePoint); + auto const cafeUGC = MakeTestUGCUpdate(Time(chrono::hours(24 * 10))); + + auto const railwayId = builder.FeatureIdForRailwayAtPoint(railwayPoint); + auto const railwayUGC = MakeTestUGCUpdate(Time(chrono::hours(48 * 10))); + + { + Storage s(builder.GetDataSource()); + s.Load(); + s.SetUGCUpdate(cafeId, cafeUGC); + s.SetUGCUpdate(railwayId, railwayUGC); + auto & indexes = s.GetIndexesForTesting(); + TEST_EQUAL(indexes.size(), 2, ()); + + auto & cafeIndex = indexes.front(); + cafeIndex.m_dataVersion = kMinVersionForMigration - 1; + cafeIndex.m_version = IndexVersion::V0; + + auto & railwayIndex = indexes.back(); + railwayIndex.m_dataVersion = kMinVersionForMigration; + railwayIndex.m_version = IndexVersion::V0; + railwayIndex.m_synchronized = true; + + s.SaveIndex(); + } + + { + Storage s(builder.GetDataSource()); + s.Load(); + // Migration should pass successfully even there are indexes with a data version lower than minimal. + // Such indexes should be marked as deleted and should be removed after the defragmentation process. + auto const & indexes = s.GetIndexesForTesting(); + TEST_EQUAL(indexes.size(), 1, ()); + + auto const & railwayIndex = indexes.back(); + TEST_EQUAL(static_cast(railwayIndex.m_version), static_cast(IndexVersion::Latest), ()); + TEST(!railwayIndex.m_synchronized, ()); + TEST_EQUAL(railwayIndex.m_dataVersion, kMinVersionForMigration, ()); + s.SaveIndex(); + } + + { + Storage s(builder.GetDataSource()); + s.Load(); + auto const & indexes = s.GetIndexesForTesting(); + TEST_EQUAL(indexes.size(), 1, ()); + auto const & railwayIndex = indexes.back(); + TEST_EQUAL(static_cast(railwayIndex.m_version), static_cast(IndexVersion::Latest), ()); + TEST(!railwayIndex.m_synchronized, ()); + TEST_EQUAL(railwayIndex.m_dataVersion, kMinVersionForMigration, ()); + } + + TEST(DeleteIndexFile(), ()); + TEST(DeleteIndexFile(IndexVersion::V0), ()); + TEST(DeleteUGCFile(), ()); + TEST(DeleteUGCFile(IndexVersion::V0), ()); +} +