Skip to content

Implement #6413 : Data pages of newly gbak restored databases should marked as "swept" [CORE6164] #8549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement #6413 : Data pages of newly gbak restored databases should marked as "swept" [CORE6164] #8549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/burp/BurpTasks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@ void RestoreRelationTask::initItem(BurpGlobals* tdgbl, Item& item)
m_masterGbl->gbl_dpb_data.begin(),
m_masterGbl->gbl_dpb_data.getCount());

dpb.deleteWithTag(isc_dpb_gbak_attach);
//dpb.deleteWithTag(isc_dpb_gbak_attach);

const UCHAR* dpbBuffer = dpb.getBuffer();
const USHORT dpbLength = dpb.getBufferLength();
Expand Down
42 changes: 39 additions & 3 deletions src/jrd/dpm.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2232,7 +2232,14 @@ void DPM_store( thread_db* tdbb, record_param* rpb, PageStack& stack, const Jrd:
memset(data + size, 0, fill);

Ods::pag* page = rpb->getWindow(tdbb).win_buffer;
if (page->pag_flags & dpg_swept)

// When restoring, mark primary data page as swept, pointer page already marked by locate_space()
if (tdbb->getAttachment()->isGbak() && !rpb->rpb_relation->isSystem() && (type == DPM_primary))
{
page->pag_flags |= dpg_swept;
CCH_RELEASE(tdbb, &rpb->getWindow(tdbb));
}
else if (page->pag_flags & dpg_swept)
{
page->pag_flags &= ~dpg_swept;
mark_full(tdbb, rpb);
Expand Down Expand Up @@ -3083,7 +3090,13 @@ static void extend_relation(thread_db* tdbb, jrd_rel* relation, WIN* window, con
UCHAR* bits = (UCHAR*) (ppage->ppg_page + dbb->dbb_dp_per_pp);
PPG_DP_BIT_CLEAR(bits, slot, PPG_DP_ALL_BITS);

if (type != DPM_primary)
if (type == DPM_primary)
{
// When restoring, mark slot as swept, data page will be marked by our caller
if (tdbb->getAttachment()->isGbak() && !relation->isSystem())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I would put this check in a separate inline function like inline bool sweptOnRestore(thread_db* tdbb, jrd_rel* rel).
  2. isGbak() is not 100% safe to use. I remember I tested a case where I created some DB trigger which inserts/updates data. And when I started gbak -b, the normal swept flag logic wasn't working during execution of the trigger. So we need to distinguish between backup and restore. isRWGbak() is more suitable here but it's not going to work right away. ATT_creator is also should be passed to parallel worker attachments. To do that, I created isc_dpb_gbak_restore_rel_attach so JProvider::internalAttach can check it along with isc_dpb_gbak_attach and set ATT_creator.

If it's needed, I can provide a patch with these corrections.

PPG_DP_BIT_SET(bits, slot, ppg_dp_swept);
}
else
PPG_DP_BIT_SET(bits, slot, ppg_dp_secondary);

for (unsigned i = 1; i < cntAlloc; i++)
Expand Down Expand Up @@ -3536,7 +3549,13 @@ static rhd* locate_space(thread_db* tdbb,

PPG_DP_BIT_CLEAR(bits, slot, ppg_dp_empty);
if (type == DPM_primary)
{
PPG_DP_BIT_CLEAR(bits, slot, ppg_dp_secondary);

// When restoring, mark slot as swept, data page will be marked by our caller
if (tdbb->getAttachment()->isGbak() && !relation->isSystem())
PPG_DP_BIT_SET(bits, slot, ppg_dp_swept);
}
else
PPG_DP_BIT_SET(bits, slot, ppg_dp_secondary);

Expand Down Expand Up @@ -3918,11 +3937,28 @@ static void store_big_record(thread_db* tdbb,
header->rhdf_b_page, header->rhdf_b_line);
#endif


bool markPP = false;

// When restoring, mark primary data page as swept, pointer page already marked by locate_space()
if (tdbb->getAttachment()->isGbak() && !rpb->rpb_relation->isSystem() && (type == DPM_primary))
{
page->dpg_header.pag_flags |= dpg_swept;
}
else if (page->dpg_header.pag_flags & dpg_swept)
{
page->dpg_header.pag_flags &= ~dpg_swept;
markPP = true;
}

if (!(page->dpg_header.pag_flags & dpg_large))
{
page->dpg_header.pag_flags |= dpg_large;
mark_full(tdbb, rpb);
markPP = true;
}

if (markPP)
mark_full(tdbb, rpb);
else
CCH_RELEASE(tdbb, &rpb->getWindow(tdbb));
}
Loading