vmware-archive / go-pmem-transaction

Golang library for using persistent memory
Other
29 stars 5 forks source link

undo transaction abort tests failing #23

Open mohit10verma opened 5 years ago

mohit10verma commented 5 years ago

After this commit, I see some "UndoLogBasic" tests in transaction/txtest are failing. To reproduce: cd txtest/; go test -run=UndoLogBasic

jerrinsg commented 5 years ago

Thanks for reporting the issue. I see two separate issues here. Issue 1 - Test "data structure commit when undoTx.Log() does update" is failing. This issue is happening because of a bug in the implementation of slice logging (1b0fe7f9). When a slice is logged, we set the value of sliceElemSize in Log() as the number of elements in the slice. This value is reset to 0 when End() is called. But if End() is not called and the transaction gets aborted instead, then we do not clear the value of sliceElemSize. If this transaction handle gets reused, and End() sees this incorrect value of sliceElemSize, it will try flushing unsafe memory addresses.

jerrinsg commented 5 years ago

Issue 2 - Test " data structure abort when undoTx.Log() does update" is failing. This is happening due to a bug in the code optimizations made as part of 489a9521. When transaction release is called, we call t.abort() only if the value of tail is greater than zero. But this prevents the value of level being reset to 0 in those cases where we begin a transaction, but did not log any data values.

mohit10verma commented 5 years ago

Jerrin, For issue 1 you report above: When the transaction is aborted, aren't we making sure in resetLogTail() method that new log entries are always created. That would make sliceElemsize entry 0. This is the piece of code I am looking at:

func initUndoTx(logHeadPtr unsafe.Pointer) unsafe.Pointer {
    undoArray = newBitmap(logNum)
    if logHeadPtr == nil {
     /* some code */
    } else {
        txHeaderPtr = (*undoTxHeader)(logHeadPtr)

                /* some code */
        // Recover data from previous pending transactions, if any
        for i := 0; i < logNum; i++ {
                         /* some code */
            tx.abort(true)
        }
    }

func (t *undoTx) abort(realloc bool) error {
    defer t.Unlock()
    // Replay undo logs. Order last updates first, during abort
        /* some code */
    t.resetLogTail(realloc)
    return nil
}

func (t *undoTx) resetLogTail(realloc bool) {

    if realloc {
        // Allocate a new backing array if realloc is true or if the array was
        // expanded to accommodate more log entries.
        t.log = pmake([]entry, NumEntries)
        runtime.PersistRange(unsafe.Pointer(&t.log), unsafe.Sizeof(t.log))
    } else {
                  /* some code */
    }
}
jerrinsg commented 5 years ago

Hi Mohit, When we call Release on an undo transaction handle (before calling End), we try to reuse the backing array - by zeroing the ptr and data entries. But in this scenario we are not clearing sliceElemsize which can cause the first issue.