KeyError in WritableTree.extend

[jrueb]

I'm repeatedly calling extend on a Tree inside a file made with uproot.recreate. At some point I can get the following exception: KeyError: 'fBasketBytes'. The traceback points to this line: https://github.com/scikit-hep/uproot4/blob/3fe33aff6d6ed88203eed90f4edb5f2bc720f3e6/src/uproot/writing/_cascadetree.py#L479

Unfortunately this happens after processing several gigabytes of private data and I'm not able to figure out a simple script to reproduce this issue. Let me try to write down what I know: I have the feeling this always happens when the if condition is true in this line: https://github.com/scikit-hep/uproot4/blob/3fe33aff6d6ed88203eed90f4edb5f2bc720f3e6/src/uproot/writing/_cascadetree.py#L472 From debugging I see self._branch_data has an element with kind equal to 'record'. This kind of element does not have fBasketBytes and thus once the for loop reaches this item (in fact it is the first item), the exception occurs. Is there maybe a check like

if datum["kind"] == "record":
    continue

missing? Is see this several times in the code, but there is no such check inside the for loop where the exception happens.

I could try to give something reproducible if I knew how to get self._num_baskets >= self._basket_capacity - 1 to true.

Uproot version 4.1.9

[jpivarski]

You're absolutely right, and I'm sorry this had to happen at the end of a GB-scale writing session.

I semi-independently came to the conclusion that 971d308c236624b772b6b83394e38da54f4ebd74 is what was needed, in that I read the first half of your report, looked into it at all the places where Tree._branch_data was used, and concluded that any extraction of its contents should first check the "kind" field. Then I saw that you had figured that out, too, so I have even more confidence that that's what went wrong. This list of dicts was intended to hold the changing state of a TBranch during the write, but then two types of such dicts were needed to handle nested data structures. You must be writing some interesting data type.

The self._num_baskets >= self._basket_capacity - 1 check that you're referring to is checking for when the TTree metadata is too small to write the locations of additional TBaskets. There's a way to set the initial number of such slots (I think it's 100), but instead of messing with that, you could write excessively small TBaskets (very small data in each call to extend), which would exhaust the number of preallocated TBasket locations. That would trigger the TTree metadata-rewrite sooner, to reproduce this bug.

[jpivarski]

I was in a hurry when I wrote the above. For additional clarification, the "TTree metadata object" contains arrays that specify the number of bytes, number of entries, and file-seek positions of all the TBaskets for each TBranch. Objects on disk can't grow without potentially overwriting the object written just after it, so we have to preallocate the TTree with a given size, and that means that the number of bytes, number of entries, and seek-position arrays have a maximum capacity. Each time you call extend, you add one TBasket to each TBranch. When adding the next TBasket would go over capacity, the TTree metadata needs to be rewritten.

Rewriting the TTree metadata doesn't mean rewriting all the previous TBaskets. What happens is that we find a new, larger block of space in the file (probably at the end of the file), allocate the bigger one, copy all the metadata to the new spot, including seek positions to the already-written TBaskets, point the TDirectory at the new TTree metadata, and then invalidate the old spot. (Yes, the file becomes fragmented, but these metadata objects are not very large. If the space can be used by a new allocation in the future, it will be, but probably not in a perfectly-fitting way.)

That's why it took so long to encounter this bug: you were probably writing reasonably-sized TBaskets, and the initial TTree metadata allocation is large. To reproduce the bug, you can write artificially small TBaskets, since the capacity gets used up by the number of TBaskets, not their individual sizes.

[jrueb]

Thanks for the explanation and the commit. For me #547 fixes it.