diff --git a/src/gtests/gtests_json.cpp b/src/gtests/gtests_json.cpp index d03a636da0..368deb544f 100644 --- a/src/gtests/gtests_json.cpp +++ b/src/gtests/gtests_json.cpp @@ -15,6 +15,8 @@ #include #include "fileio.h" +#include "indexfiles.h" +#include "index_rotator.h" #include "json/cJSON.h" #include "sphinx.h" #include "sphinxjson.h" @@ -63,6 +65,62 @@ TEST_F ( JsonFileParseTest, ValidJson ) } +TEST ( IndexFiles, ReadsVersionFromJsonHeader ) +{ + CSphString sBase; + sBase.SetSprintf ( "__indexfiles_%d_json_header", GetOsProcessId() ); + CSphString sHeader; + sHeader.SetSprintf ( "%s.sph", sBase.cstr() ); + + CSphString sError; + CSphWriterNonThrottled tWriter; + ASSERT_TRUE ( tWriter.OpenFile ( sHeader, sError ) ) << sError.cstr(); + tWriter.PutBytes ( R"({"index_format_version":67})", strlen ( R"({"index_format_version":67})" ) ); + tWriter.CloseFile(); + ASSERT_FALSE ( tWriter.IsError() ); + + IndexFiles_c tFiles ( sBase ); + ASSERT_TRUE ( tFiles.CheckHeader() ) << tFiles.ErrorMsg(); + DWORD uVersion; + ASSERT_TRUE ( tFiles.GetVersion ( uVersion ) ) << tFiles.ErrorMsg(); + EXPECT_EQ ( uVersion, 67U ); + + unlink ( sHeader.cstr() ); +} + + +TEST ( IndexRotator, IgnoresMalformedJsonNewHeader ) +{ + CSphString sBase; + sBase.SetSprintf ( "__indexfiles_%d_rotation", GetOsProcessId() ); + CSphString sHeader; + sHeader.SetSprintf ( "%s.sph", sBase.cstr() ); + CSphString sNewHeader; + sNewHeader.SetSprintf ( "%s.new.sph", sBase.cstr() ); + + auto fnWriteHeader = [] ( const CSphString & sFile, const char * sData ) + { + CSphString sError; + CSphWriterNonThrottled tWriter; + if ( !tWriter.OpenFile ( sFile, sError ) ) + return false; + tWriter.PutBytes ( sData, strlen ( sData ) ); + tWriter.CloseFile(); + return !tWriter.IsError(); + }; + + ASSERT_TRUE ( fnWriteHeader ( sHeader, R"({"index_format_version":67})" ) ); + ASSERT_TRUE ( fnWriteHeader ( sNewHeader, "{\n" ) ); + + CheckIndexRotate_c tCheck ( sBase ); + EXPECT_FALSE ( tCheck.RotateFromNew() ); + EXPECT_TRUE ( tCheck.RotateReenable() ); + + unlink ( sHeader.cstr() ); + unlink ( sNewHeader.cstr() ); +} + + TEST_F ( JsonFileParseTest, NonJsonFormat ) { Write ( "not json" ); diff --git a/src/index_rotator.cpp b/src/index_rotator.cpp index 6644027658..de71e64af1 100644 --- a/src/index_rotator.cpp +++ b/src/index_rotator.cpp @@ -15,9 +15,20 @@ #include "detail/indexlink.h" namespace { +inline bool CheckHeader ( const CSphString & sPath, const char * sType = "" ) +{ + IndexFiles_c tFiles ( sPath ); + if ( !tFiles.CheckHeader ( sType ) ) + return false; + + DWORD uVersion; + return tFiles.GetVersion ( uVersion ); +} + + inline RotateFrom_e Check ( const CSphString& sPath ) noexcept { - switch ( ( IndexFiles_c ( sPath ).CheckHeader() ? 1 : 0 ) + ( IndexFiles_c ( sPath ).CheckHeader ( ".new" ) ? 2 : 0 ) ) + switch ( ( CheckHeader ( sPath ) ? 1 : 0 ) + ( CheckHeader ( sPath, ".new" ) ? 2 : 0 ) ) { case 0: return RotateFrom_e::NONE; case 1: return RotateFrom_e::REENABLE; diff --git a/src/indexcheck.cpp b/src/indexcheck.cpp index 90d1baa641..497bff5730 100644 --- a/src/indexcheck.cpp +++ b/src/indexcheck.cpp @@ -622,6 +622,31 @@ bool DiskIndexChecker_c::Impl_c::ReadLegacyHeader ( CSphString& sError ) } +bool ReadIndexJsonHeaderVersion ( CSphVector & dData, const CSphString & sHeader, DWORD & uVersion, CSphString & sError ) +{ + using namespace bson; + + if ( sphJsonParse ( dData, sHeader, sError )!=JsonFileParse_e::OK ) + return false; + + Bson_c tBson ( dData ); + if ( tBson.IsEmpty() || !tBson.IsAssoc() ) + { + sError = "Something wrong read from json header - it is either empty, either not root object."; + return false; + } + + uVersion = (DWORD)Int ( tBson.ChildByName ( "index_format_version" ) ); + if ( uVersion<=1 || uVersion>INDEX_FORMAT_VERSION ) + { + sError.SetSprintf ( "%s is v.%u, binary is v.%u", sHeader.cstr(), uVersion, INDEX_FORMAT_VERSION ); + return false; + } + + return true; +} + + bool DiskIndexChecker_c::Impl_c::ReadHeader ( CSphString& sError ) { bool bHeaderIsJson; @@ -640,33 +665,19 @@ bool DiskIndexChecker_c::Impl_c::ReadHeader ( CSphString& sError ) auto sHeader = GetFilename ( SPH_EXT_SPH ); - const char* szHeader = sHeader.scstr(); using namespace bson; CSphVector dData; - if ( sphJsonParse ( dData, GetFilename ( SPH_EXT_SPH ), sError )!=JsonFileParse_e::OK ) + if ( !ReadIndexJsonHeaderVersion ( dData, sHeader, m_uVersion, sError ) ) return false; Bson_c tBson ( dData ); - if ( tBson.IsEmpty() || !tBson.IsAssoc() ) - { - sError = "Something wrong read from json header - it is either empty, either not root object."; - return false; - } - - // version - m_uVersion = (DWORD)Int ( tBson.ChildByName ( "index_format_version" ) ); - if ( m_uVersion <= 1 || m_uVersion > INDEX_FORMAT_VERSION ) - { - sError.SetSprintf ( "%s is v.%u, binary is v.%u", szHeader, m_uVersion, INDEX_FORMAT_VERSION ); - return false; - } // we don't support anything prior to v64 with json format DWORD uMinFormatVer = 64; if ( m_uVersion < uMinFormatVer ) { - sError.SetSprintf ( "tables prior to v.%u are no longer supported (use index_converter tool); %s is v.%u", uMinFormatVer, szHeader, m_uVersion ); + sError.SetSprintf ( "tables prior to v.%u are no longer supported (use index_converter tool); %s is v.%u", uMinFormatVer, sHeader.cstr(), m_uVersion ); return false; } diff --git a/src/indexcheck.h b/src/indexcheck.h index b0b341c0d4..ea51b87279 100644 --- a/src/indexcheck.h +++ b/src/indexcheck.h @@ -44,6 +44,9 @@ class DebugCheckError_i DebugCheckError_i* MakeDebugCheckError ( FILE* fp, DocID_t* pExtract ); +// Read and validate the format version from a JSON plain-index header. +bool ReadIndexJsonHeaderVersion ( CSphVector & dData, const CSphString & sHeader, DWORD & uVersion, CSphString & sError ); + // disk index checker class DiskIndexChecker_c { diff --git a/src/indexfiles.cpp b/src/indexfiles.cpp index c7c2ee38b2..ab7c8beee2 100644 --- a/src/indexfiles.cpp +++ b/src/indexfiles.cpp @@ -16,6 +16,7 @@ #include "fileio.h" #include "fileutils.h" #include "sphinxint.h" +#include "indexcheck.h" #include "tokenizer/tokenizer.h" static IndexFileExt_t g_dIndexFilesExts[SPH_EXT_TOTAL] = @@ -103,7 +104,7 @@ bool IndexFiles_c::HasAllFiles ( const char * sType ) { for ( const auto & dExt : g_dIndexFilesExts ) { - if ( m_uVersionINDEX_FORMAT_VERSION ) { - m_sLastError.SetSprintf ( "%s is v.%u, binary is v.%u", sPath.cstr(), uVersion, INDEX_FORMAT_VERSION ); + m_sLastError.SetSprintf ( "%s is v.%u, binary is v.%u", m_sHeaderPath.cstr(), uVersion, INDEX_FORMAT_VERSION ); return false; } m_uVersion = uVersion; @@ -296,6 +296,25 @@ bool IndexFiles_c::CheckHeader ( const char * sType ) } +bool IndexFiles_c::GetVersion ( DWORD & uVersion ) +{ + if ( !m_uVersion ) + { + if ( m_sHeaderPath.IsEmpty() && !CheckHeader() ) + return false; + + CSphVector dData; + DWORD uHeaderVersion; + if ( !ReadIndexJsonHeaderVersion ( dData, m_sHeaderPath, uHeaderVersion, m_sLastError ) ) + return false; + m_uVersion = uHeaderVersion; + } + + uVersion = *m_uVersion; + return true; +} + + bool IndexFiles_c::ReadKlistTargets ( StrVec_t & dTargets, const char * szType ) { CSphString sPath = FullPath ( sphGetExt(SPH_EXT_SPK), szType ); diff --git a/src/indexfiles.h b/src/indexfiles.h index 38b8cde46e..95b1ccbb48 100644 --- a/src/indexfiles.h +++ b/src/indexfiles.h @@ -16,6 +16,7 @@ #include "sphinxint.h" #include "indexfilebase.h" +#include #include enum ESphExt : BYTE @@ -59,16 +60,18 @@ const char* sphGetExt ( ESphExt eExt ); /// encapsulates all common actions over index files in general (copy/rename/delete etc.) class IndexFiles_c : public IndexFileBase_c { - DWORD m_uVersion = INDEX_FORMAT_VERSION; - CSphString m_sIndexName; // used for information purposes (logs) + std::optional m_uVersion; + CSphString m_sHeaderPath; + CSphString m_sIndexName; // used for information purposes (logs) CSphString m_sLastError; bool m_bFatal = false; // if fatal fail happened (unable to rename during rollback) CSphString FullPath ( const char * szExt, const CSphString& sSuffix = "", const CSphString& sBase = "" ); + DWORD GetVersionForFiles() const { return m_uVersion.value_or ( INDEX_FORMAT_VERSION ); } inline void SetName ( CSphString sIndex ) { m_sIndexName = std::move(sIndex); } public: IndexFiles_c() = default; - explicit IndexFiles_c ( CSphString sBase, const char* sIndex=nullptr, DWORD uVersion = INDEX_FORMAT_VERSION ) + explicit IndexFiles_c ( CSphString sBase, const char* sIndex=nullptr, std::optional uVersion = std::nullopt ) : IndexFileBase_c { std::move ( sBase ) } , m_uVersion ( uVersion ) { @@ -79,13 +82,14 @@ class IndexFiles_c : public IndexFileBase_c inline const char * ErrorMsg () const { return m_sLastError.cstr(); } inline bool IsFatal() const { return m_bFatal; } - // read .sph and adopt index version from there. + // check that .sph is readable and has a supported legacy header, if applicable. bool CheckHeader ( const char * sType="" ); // read the beginning of .spk and parse killlist targets bool ReadKlistTargets ( StrVec_t & dTargets, const char * sType="" ); - DWORD GetVersion() const { return m_uVersion; } + // lazily read the version from a JSON header when CheckHeader() could not obtain it. + bool GetVersion ( DWORD & uVersion ); // simple make decorated path, like '.old' -> /path/to/index.old CSphString MakePath ( const char * szSuffix = "" ); diff --git a/src/indextool.cpp b/src/indextool.cpp index 0c2c2d5f64..8217f2acdf 100644 --- a/src/indextool.cpp +++ b/src/indextool.cpp @@ -887,12 +887,16 @@ static void ApplyKilllists ( CSphConfig & hConf ) fprintf ( stdout, "WARNING: unable to index header for table %s\n", tIndex.m_sName.cstr() ); continue; } - tIndex.m_uVersion = tIndexFiles.GetVersion(); + if ( !tIndexFiles.GetVersion ( tIndex.m_uVersion ) ) + { + fprintf ( stdout, "WARNING: unable to read header version for table %s: %s\n", tIndex.m_sName.cstr(), tIndexFiles.ErrorMsg() ); + continue; + } // no lookups prior to v.54 - if ( tIndexFiles.GetVersion() < 54 ) + if ( tIndex.m_uVersion < 54 ) { - fprintf ( stdout, "WARNING: table '%s' version: %u, min supported is 54\n", tIndex.m_sName.cstr(), tIndexFiles.GetVersion() ); + fprintf ( stdout, "WARNING: table '%s' version: %u, min supported is 54\n", tIndex.m_sName.cstr(), tIndex.m_uVersion ); continue; } diff --git a/test/indextool/CMakeLists.txt b/test/indextool/CMakeLists.txt index 97bc03a13e..8f72ed909c 100644 --- a/test/indextool/CMakeLists.txt +++ b/test/indextool/CMakeLists.txt @@ -13,6 +13,13 @@ else () -P ${CMAKE_CURRENT_SOURCE_DIR}/test.cmake WORKING_DIRECTORY "${MANTICORE_BINARY_DIR}/test" ) SET_TESTS_PROPERTIES ( Perform_indextool PROPERTIES LABELS INDEXTOOL ) + add_test ( NAME Apply_killlists_with_legacy_json_header COMMAND ${CMAKE_COMMAND} + -D INDEXER=$ + -D INDEXTOOL=$ + -D LEGACY_FIXTURE=${CMAKE_SOURCE_DIR}/test/test_406/data + -P ${CMAKE_CURRENT_SOURCE_DIR}/test_apply_killlists_legacy_json.cmake + WORKING_DIRECTORY "${MANTICORE_BINARY_DIR}/test" ) + SET_TESTS_PROPERTIES ( Apply_killlists_with_legacy_json_header PROPERTIES LABELS INDEXTOOL ) endif () diff --git a/test/indextool/test_apply_killlists_legacy_json.cmake b/test/indextool/test_apply_killlists_legacy_json.cmake new file mode 100644 index 0000000000..bec4375956 --- /dev/null +++ b/test/indextool/test_apply_killlists_legacy_json.cmake @@ -0,0 +1,72 @@ +# Verify that indextool applies a current killlist to a legacy JSON plain index. +# The fixture was written in format v65, while indexer creates delta in the current format. +cmake_minimum_required ( VERSION 3.17 ) + +set ( WORKDIR "${CMAKE_CURRENT_BINARY_DIR}/indextool-killlists" ) +execute_process ( COMMAND ${CMAKE_COMMAND} -E rm -rf "${WORKDIR}" ) +execute_process ( COMMAND ${CMAKE_COMMAND} -E make_directory "${WORKDIR}" ) + +file ( COPY "${LEGACY_FIXTURE}/" DESTINATION "${WORKDIR}/fixture" ) +file ( GLOB dLegacyFiles "${WORKDIR}/fixture/index.0.*" ) +foreach ( sFile IN LISTS dLegacyFiles ) + get_filename_component ( sName "${sFile}" NAME ) + string ( REGEX REPLACE "^index\\.0" "main" sName "${sName}" ) + file ( RENAME "${sFile}" "${WORKDIR}/${sName}" ) +endforeach () + +file ( WRITE "${WORKDIR}/delta.tsv" "1\tupdated\n" ) +file ( WRITE "${WORKDIR}/manticore.conf" " +source src_delta +{ + type = tsvpipe + tsvpipe_command = cat ${WORKDIR}/delta.tsv + tsvpipe_field = title +} + +index main +{ + type = plain + path = ${WORKDIR}/main +} + +index delta +{ + type = plain + source = src_delta + path = ${WORKDIR}/delta + killlist_target = main:id +} +" ) + +execute_process ( + COMMAND "${INDEXER}" --config "${WORKDIR}/manticore.conf" delta + RESULT_VARIABLE iIndexerResult + OUTPUT_VARIABLE sIndexerOutput + ERROR_VARIABLE sIndexerError ) +if ( iIndexerResult ) + message ( FATAL_ERROR "Failed to build current-format killer: ${sIndexerOutput}${sIndexerError}" ) +endif () + +file ( READ "${WORKDIR}/main.spm" sBefore HEX ) +execute_process ( + COMMAND "${INDEXTOOL}" --config "${WORKDIR}/manticore.conf" --apply-killlists + RESULT_VARIABLE iApplyResult + OUTPUT_VARIABLE sApplyOutput + ERROR_VARIABLE sApplyError ) +if ( iApplyResult ) + message ( FATAL_ERROR "Failed to apply killlist: ${sApplyOutput}${sApplyError}" ) +endif () +file ( READ "${WORKDIR}/main.spm" sAfter HEX ) + +if ( NOT sBefore STREQUAL "00000000" OR NOT sAfter STREQUAL "01000000" ) + message ( FATAL_ERROR "Expected kill bit change 00000000 -> 01000000, got ${sBefore} -> ${sAfter}" ) +endif () + +execute_process ( + COMMAND "${INDEXTOOL}" --config "${WORKDIR}/manticore.conf" --check main + RESULT_VARIABLE iCheckResult + OUTPUT_VARIABLE sCheckOutput + ERROR_VARIABLE sCheckError ) +if ( iCheckResult ) + message ( FATAL_ERROR "Updated legacy index failed check: ${sCheckOutput}${sCheckError}" ) +endif ()