Skip to content
Permalink
Browse files
Fix #467, crash when call one PQ after another. Refactor accum cleanup
Crash revealed by snippet from test 321:
INSERT INTO pq (query, tags) VALUES ( 'test 1', 'base3 postid postid_11' );
CALL PQ ('pq_filter', ('{"title":"filter test doc1", "gid":2 }', '{"title":"filter test doc2", "gid":13}'), 1 as docs, 1 as docs_json, 1 as query );
CALL PQ ('pq', ('{"title":"testik goes on"}', '{"title":"cat but not dog"}'), 1 as docs, 1 as docs_json, 1 as query );

Here index 'pq_filter' is empty. Call PQ follows fastpath and returns 0, however RAM accum is not cleaned at the end. So, next CALL PQ to non-empty index is confused by the data already in accum and it causes crash.
  • Loading branch information
klirichek committed Sep 12, 2018
1 parent 9742f5f commit f80f8d5d7560187078868aed9a9575f4549e98aa
Showing with 32 additions and 56 deletions.
  1. +32 −56 src/sphinxrt.cpp
@@ -849,13 +849,15 @@ class RtAccum_t : public ISphRtAccum
private:
ISphRtDictWraperRefPtr_c m_pDictRt;
bool m_bReplace = false; ///< insert or replace mode (affects CleanupDuplicates() behavior)

void ResetDict ();
public:
explicit RtAccum_t ( bool bKeywordDict );
void SetupDict ( const ISphRtIndex * pIndex, CSphDict * pDict, bool bKeywordDict );
void ResetDict ();
void Sort ();

enum EWhatClear { EPartial=1, EAccum=2, ERest=4, EAll=7};
void Cleanup ( BYTE eWhat=EAll );

void AddDocument ( ISphHits * pHits, const CSphMatch & tDoc, bool bReplace, int iRowSize, const char ** ppStr, const CSphVector<DWORD> & dMvas );
RtSegment_t * CreateSegment ( int iRowSize, int iWordsCheckpoint );
void CleanupDuplicates ( int iRowSize );
@@ -1691,6 +1693,26 @@ void RtAccum_t::Sort ()
}
}

void RtAccum_t::Cleanup ( BYTE eWhat )
{
if ( eWhat & EPartial )
{
m_dAccumRows.Resize ( 0 );
m_dStrings.Resize ( 1 ); // handle dummy zero offset
m_dMvas.Resize ( 1 );
m_dPerDocHitsCount.Resize ( 0 );
ResetDict ();
}
if ( eWhat & EAccum )
m_dAccum.Resize ( 0 );
if ( eWhat & ERest )
{
SetIndex ( nullptr );
m_iAccumDocs = 0;
m_dAccumKlist.Reset ();
}
}

void RtAccum_t::AddDocument ( ISphHits * pHits, const CSphMatch & tDoc, bool bReplace, int iRowSize, const char ** ppStr, const CSphVector<DWORD> & dMvas )
{
MEMORY ( MEM_RT_ACCUM );
@@ -2804,12 +2826,8 @@ void RtIndex_t::Commit ( int * pDeleted, ISphRtAccum * pAccExt )
// empty txn, just ignore
if ( !pAcc->m_iAccumDocs && !pAcc->m_dAccumKlist.GetLength() )
{
pAcc->SetIndex ( NULL );
pAcc->m_dAccumRows.Resize ( 0 );
pAcc->m_dStrings.Resize ( 1 );
pAcc->m_dMvas.Resize ( 1 );
pAcc->m_dPerDocHitsCount.Resize ( 0 );
pAcc->ResetDict();
pAcc->SetIndex ( nullptr );
pAcc->Cleanup ( RtAccum_t::EPartial );
return;
}

@@ -2832,12 +2850,7 @@ void RtIndex_t::Commit ( int * pDeleted, ISphRtAccum * pAccExt )
#endif

// clean up parts we no longer need
pAcc->m_dAccum.Resize ( 0 );
pAcc->m_dAccumRows.Resize ( 0 );
pAcc->m_dStrings.Resize ( 1 ); // handle dummy zero offset
pAcc->m_dMvas.Resize ( 1 );
pAcc->m_dPerDocHitsCount.Resize ( 0 );
pAcc->ResetDict();
pAcc->Cleanup ( RtAccum_t::EPartial | RtAccum_t::EAccum );

// sort accum klist, too
pAcc->m_dAccumKlist.Uniq ();
@@ -2846,9 +2859,7 @@ void RtIndex_t::Commit ( int * pDeleted, ISphRtAccum * pAccExt )
CommitReplayable ( pNewSeg, pAcc->m_dAccumKlist, pDeleted );

// done; cleanup accum
pAcc->SetIndex ( NULL );
pAcc->m_iAccumDocs = 0;
pAcc->m_dAccumKlist.Reset();
pAcc->Cleanup ( RtAccum_t::ERest );
// reset accumulated warnings
CSphString sWarning;
pAcc->GrabLastWarning ( sWarning );
@@ -3225,18 +3236,7 @@ void RtIndex_t::RollBack ( ISphRtAccum * pAccExt )
if ( !pAcc )
return;

// clean up parts we no longer need
pAcc->m_dAccum.Resize ( 0 );
pAcc->m_dAccumRows.Resize ( 0 );
pAcc->m_dStrings.Resize ( 1 ); // handle dummy zero offset
pAcc->m_dMvas.Resize ( 1 );
pAcc->m_dPerDocHitsCount.Resize ( 0 );
pAcc->ResetDict();

// finish cleaning up and release accumulator
pAcc->SetIndex ( NULL );
pAcc->m_iAccumDocs = 0;
pAcc->m_dAccumKlist.Reset();
pAcc->Cleanup ();
}

bool RtIndex_t::DeleteDocument ( const SphDocID_t * pDocs, int iDocs, CSphString & sError, ISphRtAccum * pAccExt )
@@ -11851,12 +11851,7 @@ bool PercolateIndex_c::MatchDocuments ( ISphRtAccum * pAccExt, PercolateMatchRes
// empty txn or no queries just ignore
if ( !pAcc->m_iAccumDocs || !m_dStored.GetLength() )
{
pAcc->SetIndex ( NULL );
pAcc->m_dAccumRows.Resize ( 0 );
pAcc->m_dStrings.Resize ( 1 );
pAcc->m_dMvas.Resize ( 1 );
pAcc->m_dPerDocHitsCount.Resize ( 0 );
pAcc->ResetDict();
pAcc->Cleanup ();
return true;
}

@@ -11872,16 +11867,7 @@ bool PercolateIndex_c::MatchDocuments ( ISphRtAccum * pAccExt, PercolateMatchRes
SafeDelete ( pSeg );

// done; cleanup accum
pAcc->m_dAccum.Resize ( 0 );
pAcc->m_dAccumRows.Resize ( 0 );
pAcc->m_dStrings.Resize ( 1 ); // handle dummy zero offset
pAcc->m_dMvas.Resize ( 1 );
pAcc->m_dPerDocHitsCount.Resize ( 0 );
pAcc->ResetDict();

pAcc->SetIndex ( NULL );
pAcc->m_iAccumDocs = 0;
pAcc->m_dAccumKlist.Reset();
pAcc->Cleanup ();

int64_t tmEnd = sphMicroTimer();
tRes.m_tmTotal = tmEnd - tmStart;
@@ -11896,17 +11882,7 @@ void PercolateIndex_c::RollBack ( ISphRtAccum * pAccExt )
if ( !pAcc )
return;

// clean up parts we no longer need
pAcc->m_dAccum.Resize ( 0 );
pAcc->m_dAccumRows.Resize ( 0 );
pAcc->m_dStrings.Resize ( 1 ); // handle dummy zero offset
pAcc->m_dMvas.Resize ( 1 );
pAcc->m_dPerDocHitsCount.Resize ( 0 );
pAcc->ResetDict();

pAcc->SetIndex ( NULL );
pAcc->m_iAccumDocs = 0;
pAcc->m_dAccumKlist.Reset();
pAcc->Cleanup ();
}

bool PercolateIndex_c::EarlyReject ( CSphQueryContext * pCtx, CSphMatch & tMatch ) const

0 comments on commit f80f8d5

Please sign in to comment.