firebird/firebird-mirror
firebird/firebird-mirror
8
0
Fork 1
mirror of https://github.com/FirebirdSQL/firebird.git synced 2025-01-22 18:43:02 +01:00
Code Issues

Fix an issue where the garbage collection in indexes and blobs is not performed in VIO_backout

Browse Source 
The issue starts in VIO_record. There is a call record->reset(format) which clears the REC_gc_active flag by mistake. After that VIO_gc_record reuses the record which is already active.
This commit is contained in:
Ilya Eremin 2023-09-08 12:40:24 +03:00
parent 5b14baa37b
commit 4974dd13f1
5 changed files with 47 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
src/jrd/Record.h
View File

@ -29,30 +29,24 @@
namespace Jrd namespace Jrd
{ {
// Record flags
const UCHAR REC_fake_nulls = 1; // all fields simulate being NULLs
const UCHAR REC_gc_active = 2; // relation garbage collect record block in use
const UCHAR REC_undo_active = 4; // record block in use for undo purposes
class Record class Record
{ {
template <UCHAR FLAG> friend class AutoTempRecord; friend class AutoTempRecord;
public: public:
Record(MemoryPool& p, const Format* format, UCHAR flags = 0) Record(MemoryPool& p, const Format* format, const bool temp_active = false)
: m_precedence(p), m_data(p) : m_precedence(p), m_data(p), m_fake_nulls(false), m_temp_active(temp_active)
{ {
m_data.resize(format->fmt_length); m_data.resize(format->fmt_length);
m_format = format; m_format = format;
m_flags = flags;
} }
Record(MemoryPool& p, const Record* other) Record(MemoryPool& p, const Record* other)
: m_precedence(p), m_data(p, other->m_data), : m_precedence(p), m_data(p, other->m_data),
m_format(other->m_format), m_flags(other->m_flags) m_format(other->m_format), m_fake_nulls(other->m_fake_nulls), m_temp_active(false)
{} {}
void reset(const Format* format = NULL, UCHAR flags = 0) void reset(const Format* format = NULL)
{ {
if (format && format != m_format) if (format && format != m_format)
{ {
@ -60,24 +54,24 @@ namespace Jrd
m_format = format; m_format = format;
} }
m_flags = flags; m_fake_nulls = false;
} }
void setNull(USHORT id) void setNull(USHORT id)
{ {
fb_assert(!(m_flags & REC_fake_nulls)); fb_assert(!m_fake_nulls);
getData()[id >> 3] |= (1 << (id & 7)); getData()[id >> 3] |= (1 << (id & 7));
} }
void clearNull(USHORT id) void clearNull(USHORT id)
{ {
fb_assert(!(m_flags & REC_fake_nulls)); fb_assert(!m_fake_nulls);
getData()[id >> 3] &= ~(1 << (id & 7)); getData()[id >> 3] &= ~(1 << (id & 7));
} }
bool isNull(USHORT id) const bool isNull(USHORT id) const
{ {
if (m_flags & REC_fake_nulls) if (m_fake_nulls)
return true; return true;
return ((getData()[id >> 3] & (1 << (id & 7))) != 0); return ((getData()[id >> 3] & (1 << (id & 7))) != 0);
@ -91,7 +85,7 @@ namespace Jrd
memset(getData() + null_bytes, 0, getLength() - null_bytes); memset(getData() + null_bytes, 0, getLength() - null_bytes);
// Record has real NULLs now, so clear the "fake-nulls" flag // Record has real NULLs now, so clear the "fake-nulls" flag
m_flags &= ~REC_fake_nulls; m_fake_nulls = false;
} }
PageStack& getPrecedence() PageStack& getPrecedence()
@ -107,7 +101,7 @@ namespace Jrd
void copyFrom(const Record* other) void copyFrom(const Record* other)
{ {
m_format = other->m_format; m_format = other->m_format;
m_flags = other->m_flags; m_fake_nulls = other->m_fake_nulls;
copyDataFrom(other); copyDataFrom(other);
} }
@ -137,12 +131,12 @@ namespace Jrd
void fakeNulls() void fakeNulls()
{ {
m_flags |= REC_fake_nulls; m_fake_nulls = true;
} }
bool isNull() const bool isNull() const
{ {
return (m_flags & REC_fake_nulls); return m_fake_nulls;
} }
ULONG getLength() const ULONG getLength() const
@ -160,9 +154,14 @@ namespace Jrd
return m_data.begin(); return m_data.begin();
} }
bool testFlags(UCHAR mask) const bool isTempActive() const
{ {
return ((m_flags & mask) != 0); return m_temp_active;
}
void setTempActive()
{
m_temp_active = true;
} }
TraNumber getTransactionNumber() const TraNumber getTransactionNumber() const
@ -179,13 +178,13 @@ namespace Jrd
PageStack m_precedence; // stack of higher precedence pages/transactions PageStack m_precedence; // stack of higher precedence pages/transactions
Firebird::Array<UCHAR> m_data; // space for record data Firebird::Array<UCHAR> m_data; // space for record data
const Format* m_format; // what the data looks like const Format* m_format; // what the data looks like
UCHAR m_flags; // misc record flags TraNumber m_transaction_nr; // transaction number for a record
TraNumber m_transaction_nr; // transaction number for a record bool m_fake_nulls; // all fields simulate being NULLs
bool m_temp_active; // record block in use for garbage collection or undo purposes
}; };
// Wrapper for reusable temporary records // Wrapper for reusable temporary records
template <UCHAR FLAG>
class AutoTempRecord class AutoTempRecord
{ {
public: public:
@ -193,7 +192,7 @@ namespace Jrd
: m_record(record) : m_record(record)
{ {
// validate record and its flag // validate record and its flag
fb_assert(!record || (record->m_flags & FLAG)); fb_assert(!record || record->m_temp_active);
} }
~AutoTempRecord() ~AutoTempRecord()
@ -206,7 +205,7 @@ namespace Jrd
// class object can be initialized just once // class object can be initialized just once
fb_assert(!m_record); fb_assert(!m_record);
// validate record and its flag // validate record and its flag
fb_assert(!record || (record->m_flags & FLAG)); fb_assert(!record || record->m_temp_active);
m_record = record; m_record = record;
return m_record; return m_record;
@ -216,7 +215,8 @@ namespace Jrd
{ {
if (m_record) if (m_record)
{ {
m_record->m_flags &= ~FLAG; fb_assert(m_record->m_temp_active);
m_record->m_temp_active = false;
m_record = NULL; m_record = NULL;
} }
} }
@ -234,9 +234,6 @@ namespace Jrd
private: private:
Record* m_record; Record* m_record;
}; };
typedef AutoTempRecord<REC_gc_active> AutoGCRecord;
typedef AutoTempRecord<REC_undo_active> AutoUndoRecord;
} // namespace } // namespace
#endif // JRD_RECORD_H #endif // JRD_RECORD_H

8
src/jrd/Savepoint.cpp
View File

@ -86,7 +86,7 @@ void VerbAction::garbageCollectIdxLite(thread_db* tdbb, jrd_tra* transaction, SI
rpb.rpb_transaction_nr = transaction->tra_number; rpb.rpb_transaction_nr = transaction->tra_number;
Record* next_ver; Record* next_ver;
AutoUndoRecord undo_next_ver(transaction->findNextUndo(this, vct_relation, recordNumber)); AutoTempRecord undo_next_ver(transaction->findNextUndo(this, vct_relation, recordNumber));
AutoPtr<Record> real_next_ver; AutoPtr<Record> real_next_ver;
next_ver = undo_next_ver; next_ver = undo_next_ver;
@ -108,7 +108,7 @@ void VerbAction::garbageCollectIdxLite(thread_db* tdbb, jrd_tra* transaction, SI
BUGCHECK(185); // msg 185 wrong record version BUGCHECK(185); // msg 185 wrong record version
Record* prev_ver = NULL; Record* prev_ver = NULL;
AutoUndoRecord undo_prev_ver; AutoTempRecord undo_prev_ver;
AutoPtr<Record> real_prev_ver; AutoPtr<Record> real_prev_ver;
if (nextAction && nextAction->vct_undo && nextAction->vct_undo->locate(recordNumber)) if (nextAction && nextAction->vct_undo && nextAction->vct_undo->locate(recordNumber))
@ -197,7 +197,7 @@ void VerbAction::mergeTo(thread_db* tdbb, jrd_tra* transaction, VerbAction* next
// because going version for sure has all index entries successfully set up (in contrast with undo) // because going version for sure has all index entries successfully set up (in contrast with undo)
// we can use lightweigth version of garbage collection without collection of full staying list // we can use lightweigth version of garbage collection without collection of full staying list
AutoUndoRecord this_ver(item.setupRecord(transaction)); AutoTempRecord this_ver(item.setupRecord(transaction));
garbageCollectIdxLite(tdbb, transaction, recordNumber, nextAction, this_ver); garbageCollectIdxLite(tdbb, transaction, recordNumber, nextAction, this_ver);
@ -313,7 +313,7 @@ void VerbAction::undo(thread_db* tdbb, jrd_tra* transaction, bool preserveLocks,
} }
else else
{ {
AutoUndoRecord record(vct_undo->current().setupRecord(transaction)); AutoTempRecord record(vct_undo->current().setupRecord(transaction));
Record* const save_record = rpb.rpb_record; Record* const save_record = rpb.rpb_record;
record_param new_rpb = rpb; record_param new_rpb = rpb;

2
src/jrd/idx.cpp
View File

@ -554,7 +554,7 @@ bool IndexCreateTask::handler(WorkItem& _item)
// Checkout a garbage collect record block for fetching data. // Checkout a garbage collect record block for fetching data.
AutoGCRecord gc_record(VIO_gc_record(tdbb, relation)); AutoTempRecord gc_record(VIO_gc_record(tdbb, relation));
if (m_flags & IS_LARGE_SCAN) if (m_flags & IS_LARGE_SCAN)
{ {

7
src/jrd/tra.h
View File

@ -364,15 +364,16 @@ public:
Record* const record = *iter; Record* const record = *iter;
fb_assert(record); fb_assert(record);
if (!record->testFlags(REC_undo_active)) if (!record->isTempActive())
{ {
// initialize record for reuse // initialize record for reuse
record->reset(format, REC_undo_active); record->reset(format);
record->setTempActive();
return record; return record;
} }
} }
Record* const record = FB_NEW_POOL(*tra_pool) Record(*tra_pool, format, REC_undo_active); Record* const record = FB_NEW_POOL(*tra_pool) Record(*tra_pool, format, true);
tra_undo_records.add(record); tra_undo_records.add(record);
return record; return record;

21
src/jrd/vio.cpp
View File

@ -775,7 +775,7 @@ inline void clearRecordStack(RecordStack& stack)
{ {
Record* r = stack.pop(); Record* r = stack.pop();
// records from undo log must not be deleted // records from undo log must not be deleted
if (!r->testFlags(REC_undo_active)) if (!r->isTempActive())
delete r; delete r;
} }
} }
@ -877,8 +877,8 @@ void VIO_backout(thread_db* tdbb, record_param* rpb, const jrd_tra* transaction)
Record* data = NULL; Record* data = NULL;
Record* old_data = NULL; Record* old_data = NULL;
AutoGCRecord gc_rec1; AutoTempRecord gc_rec1;
AutoGCRecord gc_rec2; AutoTempRecord gc_rec2;
bool samePage; bool samePage;
bool deleted; bool deleted;
@ -2860,10 +2860,11 @@ Record* VIO_gc_record(thread_db* tdbb, jrd_rel* relation)
Record* const record = *iter; Record* const record = *iter;
fb_assert(record); fb_assert(record);
if (!record->testFlags(REC_gc_active)) if (!record->isTempActive())
{ {
// initialize record for reuse // initialize record for reuse
record->reset(format, REC_gc_active); record->reset(format);
record->setTempActive();
return record; return record;
} }
} }
@ -2871,7 +2872,7 @@ Record* VIO_gc_record(thread_db* tdbb, jrd_rel* relation)
// Allocate a garbage collect record block if all are active // Allocate a garbage collect record block if all are active
Record* const record = FB_NEW_POOL(*relation->rel_pool) Record* const record = FB_NEW_POOL(*relation->rel_pool)
Record(*relation->rel_pool, format, REC_gc_active); Record(*relation->rel_pool, format, true);
relation->rel_gc_records.add(record); relation->rel_gc_records.add(record);
return record; return record;
} }
@ -3274,7 +3275,7 @@ bool VIO_modify(thread_db* tdbb, record_param* org_rpb, record_param* new_rpb, j
if (org_rpb->rpb_runtime_flags & (RPB_refetch | RPB_undo_read)) if (org_rpb->rpb_runtime_flags & (RPB_refetch | RPB_undo_read))
{ {
const bool undo_read = (org_rpb->rpb_runtime_flags & RPB_undo_read); const bool undo_read = (org_rpb->rpb_runtime_flags & RPB_undo_read);
AutoGCRecord old_record; AutoTempRecord old_record;
if (undo_read) if (undo_read)
{ {
old_record = VIO_gc_record(tdbb, relation); old_record = VIO_gc_record(tdbb, relation);
@ -5563,7 +5564,7 @@ static UndoDataRet get_undo_data(thread_db* tdbb, jrd_tra* transaction,
rpb->rpb_runtime_flags |= RPB_undo_data; rpb->rpb_runtime_flags |= RPB_undo_data;
CCH_RELEASE(tdbb, &rpb->getWindow(tdbb)); CCH_RELEASE(tdbb, &rpb->getWindow(tdbb));
AutoUndoRecord undoRecord(undo.setupRecord(transaction)); AutoTempRecord undoRecord(undo.setupRecord(transaction));
Record* const record = VIO_record(tdbb, rpb, undoRecord->getFormat(), pool); Record* const record = VIO_record(tdbb, rpb, undoRecord->getFormat(), pool);
record->copyFrom(undoRecord); record->copyFrom(undoRecord);
@ -6458,7 +6459,7 @@ static void purge(thread_db* tdbb, record_param* rpb)
// the record. // the record.
record_param temp = *rpb; record_param temp = *rpb;
AutoGCRecord gc_rec(VIO_gc_record(tdbb, relation)); AutoTempRecord gc_rec(VIO_gc_record(tdbb, relation));
Record* record = rpb->rpb_record = gc_rec; Record* record = rpb->rpb_record = gc_rec;
VIO_data(tdbb, rpb, relation->rel_pool); VIO_data(tdbb, rpb, relation->rel_pool);
@ -6807,7 +6808,7 @@ void VIO_update_in_place(thread_db* tdbb,
// becomes meaningless. What we need to do is replace the old "delta" record // becomes meaningless. What we need to do is replace the old "delta" record
// with an old "complete" record, update in placement, then delete the old delta record // with an old "complete" record, update in placement, then delete the old delta record
AutoGCRecord gc_rec; AutoTempRecord gc_rec;
record_param temp2; record_param temp2;
const Record* prior = org_rpb->rpb_prior; const Record* prior = org_rpb->rpb_prior;