Skip to content

Fixed bug gh-9017 : Hard-committed transaction causes false "update conflicts" on SuperClassic - #9264

Open
hvlad wants to merge 3 commits into
v5.0-releasefrom
work/gh-9017-fix
Open

hvlad wants to merge 3 commits into
v5.0-releasefrom
work/gh-9017-fix

Conversation

@hvlad

@hvlad hvlad commented Nov 2, 2026

Copy link
Copy Markdown
Member

No description provided.

@hvlad

hvlad commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

This PR contains no modifications of Trace API that was used when bug was investigated.

Comment thread src/jrd/tra.cpp Resolved
Comment thread src/jrd/tpc.cpp
Comment on lines +391 to +399
bool fileExists = false;
if (!cache->m_exclusive)
{
PathName fullName = makeSharedMemoryFileName(dbb, blockNumber, true);
struct stat fileStat;

if (os_utils::stat(fullName.c_str(), &fileStat) == 0)
fileExists = (fileStat.st_size >= blockSize);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is possible that two threads can see that file is not exists, it means at shared memory initialization they both will memset this block. There is possibility for race condition, where one thread is successfully init the shmem and managed to write CN into it, then second thread is awakening and starting to do it's own init process with another one memset due to fileExists == true.

@hvlad hvlad Nov 4, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This should be prevented in SharedMemoryBase ctor.
I.e. no more than one process could initialise same mapped memory.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, indeed, I missed that.
But I don't know, can a thread fall a sleep for enough time so other attachment will do all it work and leave? I don't believe that this can happen, but who knows...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure I got you here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two attachments starting new transaction, and happens that both these transactions a heading into new TSB that is no existing yet. So, both detect that the TSB has not been created (fileExists == false). Scheduler send one of the thread to rest. Other attachment create TSB, init it with zeros, insert some records, commit, exit. The thread that was in sleep mode wakes up and begins to initialize the shared file; the file lock is unlocked, so we get initFlag == true, and because of fileExists == false, we are memset file to zero, erasing CN from the previous attachment.

And I said I didn’t believe such a long sleep would be possible under normal conditions.

Comment thread src/jrd/tpc.cpp
Comment on lines 159 to 166
@@ -157,41 +165,31 @@ void TipCache::finalizeTpc(thread_db* tdbb)
} while (m_blocks_memory.getNext());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inside TipCache::finalizeTpc() you first delete TSB that in you cache (m_blocks_memory), and then deletes all other TSB that is left on file system (releaseSharedMemory()) by looking from the newest block number and until you find a gap (e.g. starts from block 5, down to block 3 because file for that block is not existing, so we deleting block 5 and 4), but this logic is broken because you firstly delete TSB in cache.
For example:

  • att 1 is persistent connection;
  • att 2 is creating a lot of transaction, so TSB 1, 2, 3, 4 is created, then att 2 is leaving;
  • att 3 is connected, so at transaction start it will read only TSB 4 and put it in cache;
  • att 1 is disconnected;
  • att 3 is disconnected, so it will execute finalizeTpc() by firstly drop TSB 4, then start doing iteration over TSB on file system by doing lookup for TSB 4 first, it doesn't find it on disk, so basically cleanup for TSB on FS is over, so TSB 1, 2, 3 will stay on disk forever;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When att3 is connected and started transaction, it will:
a) advance OIT and cleanup old TSBs, or
b) does not advance OIT and map all TSBs into own process.

@TreeHunter9 TreeHunter9 Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a) advance OIT and cleanup old TSBs

We cannot do this because att 1 is still there.

b) does not advance OIT and map all TSBs into own process.

As I see mapInventoryPages() will work incorrectly in this case. The latest_transaction_id will contain very old value (from loadInventoryPages() at att 1 connection), so we will map only the oldest TSB. Later at transaction start we will map the newest TSB, so local cache will contain TSB 0 and TSB 5 only.

I run the example again, and the problem is still there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But when I attached to the database after my test, the a) advance OIT and cleanup old TSBs, happens and after disconnect every TSB is removed.

So I guess it is not big of a deal then.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

b) does not advance OIT and map all TSBs into own process.

As I see mapInventoryPages() will work incorrectly in this case. The latest_transaction_id will contain very old value (from loadInventoryPages() at att 1 connection), so we will map only the oldest TSB.

Looks like this should be fixed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

Comment thread src/jrd/tpc.cpp Resolved
Comment thread src/jrd/tpc.cpp Resolved
@TreeHunter9

Copy link
Copy Markdown
Contributor

For some reason I cannot resolve threads, so if you see that thread is come to an end, resolve it please for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hard-committed transaction causes false "update conflicts" on SuperClassic (5.0.3 and 5.0.4)

2 participants