Skip to content
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

Jump to bottom

Fix for issue #114 #118

Merged
merged 1 commit into from Feb 1, 2020
Merged

Fix for issue #114 #118

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 33 additions & 0 deletions CHANGELOG
View file
Open in desktop
@@ -1,5 +1,33 @@
URGENT:

TODO NOW:
- Fix #114:
- Assure Test_021_::testThatLastCommitPrevails() works
- Simplify to read RootPage into a single objects (as all other pages in the DB!!!
- Merge!

- For other problems:
- Fix min/max TX id in FSM, this is broken!!!!
- Retest with wait(10) after commit() -> Seems to work, test with Peptide!
- Check for other (wrong) usages of TX-ID
- Try:
- Allow only single concurrent reader
- Flush after every page write! Flush after every write!!!! Just for testing!
- Double check usage/rollback of retry() in concurrent test write()
- close/reopen file after each commit() -> Flush problem??
- Generally check flush
-
// why do we revert?
// After a failed optimistic verification, we only need to
// clean up the TxOidRegistry (if a anything).
//
// Nevertheless, revrt() should work, but that is a diufferent story:
// -> Brute force test by enforcing revert before/after every commit?
// System.out.println("RootRevert: " + rootPageID + " by " + this); // TODO




SessionManager::commitInfrastructure():290
- Think about handling retained objects!
-> Can we ignore this for now (no read-consistency check)?
Expand All @@ -21,6 +49,11 @@ Test:

CHANGELOG

2020-Feb-01
===========
- T.Zaeschke
- Issue #114: ZooDB may choose wrong root page when opening database.

2020-Jan-05
===========
- T.Zaeschke
Expand Down
49 changes: 21 additions & 28 deletions src/org/zoodb/internal/server/DiskAccessOneFile.java
View file
Open in desktop
Expand Up @@ -70,7 +70,7 @@
* =======
* Some data data is read on start-up and kept in memory:
* - Schema index
* - OID Index (all OIDs) TODO: needs to be changed
* - OID Index (all OIDs) TODO: needs to be changed
*
*
* Page chaining
Expand All @@ -88,13 +88,13 @@
* empty and append object data. OR: Allow fragmentation.
*
* Problem #2: How to find the empty page? Follow through all other pages? No!
* Solution #2: Keep a list of all the last pages for each data type. Could become a list of
* Solution #2: Keep a list of all the last pages for each data type. Could become a list of
* all empty pages.
*
* Alternatives:
* - Keep an index of all pages for a specific class.
* -- Abuse OID index as such an index?? -- Not very efficient.
* -- Keep one OID index per class??? Works well with few classes...
* -- Abuse OID index as such an index?? -- Not very efficient.
* -- Keep one OID index per class??? Works well with few classes...
*
*
* Advantages of paging:
Expand Down Expand Up @@ -274,10 +274,10 @@ public ObjectWriter getWriter(ZooClassDef def) {

/**
* Read objects.
* Only required for queries without index, which is worth a warning anyway.
* SEE oidIterator()!
* @param schemaId Schema ID
* @param loadFromCache Whether to load data from cache, if possible
* Only required for queries without index, which is worth a warning anyway.
* SEE oidIterator()!
* @param schemaId Schema ID
* @param loadFromCache Whether to load data from cache, if possible
*/
@Override
public CloseableIterator<ZooPC> readAllObjects(long schemaId, boolean loadFromCache) {
Expand All @@ -304,10 +304,10 @@ public CloseableIterator<ZooPC> readObjectFromIndex(

/**
* Read objects.
* Only required for queries without index, which is worth a warning anyway.
* SEE readAllObjects()!
* @param clsPx ClassProxy
* @param subClasses whether to include subclasses
* Only required for queries without index, which is worth a warning anyway.
* SEE readAllObjects()!
* @param clsPx ClassProxy
* @param subClasses whether to include subclasses
*/
@Override
public CloseableIterator<ZooHandleImpl> oidIterator(ZooClassProxy clsPx, boolean subClasses) {
Expand All @@ -321,7 +321,7 @@ public CloseableIterator<ZooHandleImpl> oidIterator(ZooClassProxy clsPx, boolean

/**
* Locate an object.
* @param oid Object ID
* @param oid Object ID
* @return Path name of the object (later: position of obj)
*/
@Override
Expand All @@ -334,7 +334,7 @@ public ZooPC readObject(long oid) {

/**
* Locate an object.
* @param pc Hollow Object to read
* @param pc Hollow Object to read
*/
@Override
public ServerResponse readObject(ZooPC pc) {
Expand Down Expand Up @@ -384,8 +384,8 @@ public GenericObject readGenericObject(ZooClassDef def, long oid) {
* Locate an object. This version allows providing a data de-serializer. This will be handy
* later if we want to implement some concurrency, which requires using multiple of the
* stateful DeSerializers.
* @param dds DataDeSerializer
* @param oid Object ID
* @param dds DataDeSerializer
* @param oid Object ID
* @return Path name of the object (later: position of obj)
*/
@Override
Expand Down Expand Up @@ -591,25 +591,18 @@ public void revert() {
//Empty file buffers. For now we just flush them.
file.flush(); //TODO revert for file???

RootPage rootPage = sm.getRootPage();
RootPage rootPage = sm.getCurrentRootPage();
//revert --> back to previous (expected) schema-tx-ID
schemaIndex.revert(rootPage.getSchemIndexPage(), txContext.getSchemaTxId());
schemaIndex.revert(rootPage.getSchemaIndexPage(), txContext.getSchemaTxId());
//We use the historic page count to avoid page-leaking
freeIndex.revert(rootPage.getFMSPage(), rootPage.getFSMPageCount());
//We do NOT reset the OID count. That may cause OID leaking(does it?), but the OIDs are
//still assigned to uncommitted objects.
oidIndex.revert(rootPage.getOidIndexPage());

// WE should definitely:
// sm.txManager.deRegisterTx(txId);
//
// Should we also: ???
// txContext.reset();
//
//
// If rollback() would use revert() (why doesn't it?),
// we could test this by rolling back a modification, modifying it in another TX/PM/PMF,
// and then attempt modifying it again.
// More cleanup
sm.getTxManager().deRegisterTx(txId);
txContext.reset();
}

/**
Expand Down
125 changes: 121 additions & 4 deletions src/org/zoodb/internal/server/RootPage.java
View file
Open in desktop
Expand Up @@ -20,16 +20,106 @@
*/
package org.zoodb.internal.server;

import org.zoodb.internal.server.DiskIO.PAGE_TYPE;

final class RootPage {

private long txId;
private int userPage;
private int oidPage;
private int schemaPage;
private int indexPage;
private int freeSpaceIndexPage;
private int pageCount;
private long commitId;

private long lastUsedOID;
private int lastUsedPageId;

public static RootPage read(StorageChannelInput in, int pageId) {
RootPage page = new RootPage();

page.lastUsedPageId = pageId;
in.seekPageForRead(PAGE_TYPE.ROOT_PAGE, pageId);
//read main directory (page IDs)
//commit ID
page.txId = in.readLong();
//User table
page.userPage = in.readInt();
//OID table
page.oidPage = in.readInt();
//schemata
page.schemaPage = in.readInt();
//indices
page.indexPage = in.readInt();
//free space index
page.freeSpaceIndexPage = in.readInt();
//page count (required for recovery of crashed databases)
page.pageCount = in.readInt();
//last used oid - this may be larger than the last stored OID if the last object was deleted
page.lastUsedOID = in.readLong();

// The idea is that we assume a page has been fully written if the first and last
// part has been written (i.e. txIds are identical).
long txId2 = in.readLong();
if (page.txId != txId2) {
SessionManager.LOGGER.error("Main page is faulty: {}. Will recover from previous " +
"page version.", pageId);
page.commitId = SessionManager.ID_FAULTY_PAGE;
}

// TODO move inside protected block. This should still be save though,
// at least much better than before. Issues #114/ #116
page.commitId = in.readLong();
// TODO remove with new DB format:
// The database may not have a commitIf yet, i.e. 0.5.2 and earlier.
// Note that this change is backwards compatible, a DB written with 0.5.3
// can still be read with 0.5.2, but that will of course reintroduce the bug.
if (page.commitId == 0) {
page.commitId = page.txId;
}
return page;
}

/**
* Writes the main page.
* @param commitId Id/count of the current commit.
* @param txId Transaction ID.
* @param out Write channel.
* @param pageId file page ID
*/
public void write(long commitId, long txId, StorageChannelOutput out, int pageId) {
out.seekPageForWrite(PAGE_TYPE.ROOT_PAGE, pageId);
lastUsedPageId = pageId;

this.commitId = commitId;
this.txId = txId;

boolean isDirty(int userPage, int oidPage, int schemaPage, int indexPage,
//tx ID
out.writeLong(txId);
//User table
out.writeInt(userPage);
//OID table
out.writeInt(oidPage);
//schemata
out.writeInt(schemaPage);
//indices
out.writeInt(indexPage);
//free space index
out.writeInt(freeSpaceIndexPage);
//page count
out.writeInt(pageCount);
//last used oid
out.writeLong(lastUsedOID);
//tx ID. Writing the tx ID twice should ensure that the data between the two has been
//written correctly.
out.writeLong(txId);
// commit ID, TODO move inside block
// TODO make block larger for future changes?
out.writeLong(commitId);
}

boolean hasChanged(int userPage, int oidPage, int schemaPage, int indexPage,
int freeSpaceIndexPage) {
if (this.userPage != userPage ||
this.oidPage != oidPage ||
Expand All @@ -41,12 +131,13 @@ boolean isDirty(int userPage, int oidPage, int schemaPage, int indexPage,
return false;
}

void set(int userPage, int oidPage, int schemaPage, int indexPage,
void set(int userPage, int oidPage, int schemaPage, int indexPage, long lastUsedOID,
int freeSpaceIndexPage, int pageCount) {
this.userPage = userPage;
this.oidPage = oidPage;
this.schemaPage = schemaPage;
this.indexPage = indexPage;
this.lastUsedOID = lastUsedOID;
this.freeSpaceIndexPage = freeSpaceIndexPage;
this.pageCount = pageCount;
}
Expand All @@ -59,13 +150,13 @@ int getUserPage() {
/**
*
* @return Index page.
* @deprecated This should probably be removed. Are we gonna use this at some point?
* @deprecated This should probably be removed. Are we going to use this at some point?
*/
int getIndexPage() {
return indexPage;
}

public int getSchemIndexPage() {
public int getSchemaIndexPage() {
return schemaPage;
}

Expand All @@ -80,4 +171,30 @@ public int getFMSPage() {
public int getFSMPageCount() {
return pageCount;
}

public long getLastUsedOID() {
return lastUsedOID;
}

public long getCommitId() {
return commitId;
}

public long getTxID() {
return txId;
}

@Override
public String toString() {
return "RootPage: lastUsedPageId=" + lastUsedPageId +
" tx=" + txId +
" user=" + userPage +
" OID table=" + oidPage +
" schemata=" + schemaPage +
" indices=" + indexPage +
" FSM=" + freeSpaceIndexPage +
" pCnt=" + pageCount +
" luOID=" + lastUsedOID +
" commit=" + commitId;
}
}