Commit a7bf0311 authored by Kirill Smelkov's avatar Kirill Smelkov

wcfs: Fix crash if on invalidation handledδZ needs to access ZODB

The invalidation logic is generally right, but invalidateBlk -> ΔFtail.BlkRevAt
was being called with ctx without transaction. As the result it was
panicking as

    panic: transaction: no current transaction

    goroutine 41 [running]:
    lab.nexedi.com/kirr/neo/go/transaction.currentTxn({0x9696d8, 0xc0000d8080})
            /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/transaction/transaction.go:59 +0x77
    lab.nexedi.com/kirr/neo/go/transaction.Current(...)
            /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/transaction/api.go:206
    lab.nexedi.com/kirr/neo/go/zodb.(*Connection).checkTxnCtx(...)
            /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/connection.go:374
    lab.nexedi.com/kirr/neo/go/zodb.(*Connection).Get(0xc00010c640, {0x9696d8, 0xc0000d8080}, 0x4)
            /home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/connection.go:331 +0x73
    lab.nexedi.com/nexedi/wendelin.core/wcfs/internal/zdata.(*ΔFtail).BlkRevAt(0xc000077d40, {0x9696d8, 0xc0000d8080}, 0xc000064f60, 0x0, 0x3e5983329bbd100)
            /home/kirr/src/neo/src/lab.nexedi.com/nexedi/wendelin.core/wcfs/internal/zdata/δftail.go:1140 +0x39d
    main.(*BigFile).invalidateBlk.func1(0xc000164400, {0x9696d8, 0xc0000d8080}, 0xc0005a0000, 0x200000, 0x200000, {0xc0005a0000, 0x200000, 0x200000})
            /home/kirr/src/neo/src/lab.nexedi.com/nexedi/wendelin.core/wcfs/wcfs.go:1089 +0xb8
    main.(*BigFile).invalidateBlk(0xc000164400, {0x9696d8, 0xc0000d8080}, 0x0)
            /home/kirr/src/neo/src/lab.nexedi.com/nexedi/wendelin.core/wcfs/wcfs.go:1105 +0x3bb
    main.(*Root).handleδZ.func3({0x9696d8, 0xc0000d8080})
            /home/kirr/src/neo/src/lab.nexedi.com/nexedi/wendelin.core/wcfs/wcfs.go:898 +0x34
    lab.nexedi.com/kirr/go123/xsync.(*WorkGroup).Go.func1()
            /home/kirr/src/neo/src/lab.nexedi.com/kirr/go123/xsync/xsync.go:86 +0x68
    created by lab.nexedi.com/kirr/go123/xsync.(*WorkGroup).Go
            /home/kirr/src/neo/src/lab.nexedi.com/kirr/go123/xsync/xsync.go:83 +0x92

on any new change to tracked file block whose previous history is not covered by ΔFtail/ΔBtail.

Problem reported by @Francois.
parent c9f64495
// Copyright (C) 2018-2021 Nexedi SA and Contributors. // Copyright (C) 2018-2022 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com> // Kirill Smelkov <kirr@nexedi.com>
// //
// This program is free software: you can Use, Study, Modify and Redistribute // This program is free software: you can Use, Study, Modify and Redistribute
...@@ -868,6 +868,13 @@ retry: ...@@ -868,6 +868,13 @@ retry:
log.Infof("\n\n") log.Infof("\n\n")
} }
// put zhead's transaction into ctx because we will potentially need to
// access ZODB when processing invalidations.
// TODO better ctx = transaction.PutIntoContext(ctx, txn)
ctx0 := ctx
ctx, cancel := xcontext.Merge(ctx0, zhead.TxnCtx)
defer cancel()
// invalidate kernel cache for file data // invalidate kernel cache for file data
wg := xsync.NewWorkGroup(ctx) wg := xsync.NewWorkGroup(ctx)
for foid, δfile := range δF.ByFile { for foid, δfile := range δF.ByFile {
...@@ -920,12 +927,14 @@ retry: ...@@ -920,12 +927,14 @@ retry:
// 1. abort old and resync to new txn/at // 1. abort old and resync to new txn/at
transaction.Current(zhead.TxnCtx).Abort() transaction.Current(zhead.TxnCtx).Abort()
_, ctx = transaction.New(context.Background()) _, txnCtx := transaction.New(context.Background())
ctx, cancel = xcontext.Merge(ctx0, txnCtx) // TODO better transaction.PutIntoContext
defer cancel()
err = zhead.Resync(ctx, δZ.Tid) err = zhead.Resync(ctx, δZ.Tid)
if err != nil { if err != nil {
return err return err
} }
zhead.TxnCtx = ctx zhead.TxnCtx = txnCtx
// 2. restat invalidated ZBigFile // 2. restat invalidated ZBigFile
// NOTE no lock needed since .blksize and .size are constant during lifetime of one txn. // NOTE no lock needed since .blksize and .size are constant during lifetime of one txn.
......
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
# Copyright (C) 2018-2021 Nexedi SA and Contributors. # Copyright (C) 2018-2022 Nexedi SA and Contributors.
# Kirill Smelkov <kirr@nexedi.com> # Kirill Smelkov <kirr@nexedi.com>
# #
# This program is free software: you can Use, Study, Modify and Redistribute # This program is free software: you can Use, Study, Modify and Redistribute
...@@ -395,7 +395,7 @@ class tWCFS(_tWCFS): ...@@ -395,7 +395,7 @@ class tWCFS(_tWCFS):
class tDB(tWCFS): class tDB(tWCFS):
@func @func
def __init__(t): def __init__(t, _with_old_data=False):
t.root = testdb.dbopen() t.root = testdb.dbopen()
def _(): # close/unlock db if __init__ fails def _(): # close/unlock db if __init__ fails
exc = sys.exc_info()[1] exc = sys.exc_info()[1]
...@@ -432,9 +432,15 @@ class tDB(tWCFS): ...@@ -432,9 +432,15 @@ class tDB(tWCFS):
t._maintid = gettid() t._maintid = gettid()
# prepare initial objects for test: zfile, nonzfile # prepare initial objects for test: zfile, nonzfile
t.root['!file'] = t.nonzfile = Persistent() if not _with_old_data:
t.root['zfile'] = t.zfile = ZBigFile(blksize) t.root['!file'] = t.nonzfile = Persistent()
t.at0 = t.commit() t.root['zfile'] = t.zfile = ZBigFile(blksize)
t.at0 = t.commit()
else:
t.at0 = tAt(t, t.tail)
t.nonzfile = t.root['!file']
t.zfile = t.root['zfile']
@property @property
def head(t): def head(t):
...@@ -1318,6 +1324,37 @@ def test_wcfs_basic_read_aftertail(): ...@@ -1318,6 +1324,37 @@ def test_wcfs_basic_read_aftertail():
assert _(100*blksize) == b'' assert _(100*blksize) == b''
# verify that wcfs does not panic with "no current transaction" when processing
# invalidations if it needs to access ZODB during handleδZ.
@func
def test_wcfs_basic_invalidation_wo_dFtail_coverage():
# prepare initial data with single change to zfile[0].
@func
def _():
t = tDB(); zf = t.zfile
defer(t.close)
t.commit(zf, {0:'a'})
_()
# start wcfs with ΔFtail/ΔBtail not covering that initial change.
t = tDB(_with_old_data=True); zf = t.zfile
defer(t.close)
f = t.open(zf)
t.commit(zf, {1:'b1'}) # arbitrary commit to non-0 blk
f._assertBlk(0, 'a') # [0] becomes tracked (don't verify against computed dataok due to _with_old_data)
# wcfs was crashing on processing further invalidation to blk 0 because
# - ΔBtail.GetAt([0], head) returns valueExact=false, and so
# - ΔFtail.BlkRevAt activates "access ZODB" codepath,
# - but handleδZ was calling ΔFtail.BlkRevAt without properly putting zhead's transaction into ctx.
# -> panic.
t.commit(zf, {0:'a2'})
# just in case
f.assertBlk(0, 'a2')
# ---- verify wcfs functionality that depends on isolation protocol ---- # ---- verify wcfs functionality that depends on isolation protocol ----
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment