On 2010-01-22 16:15, Isidor Zeuner wrote:
[1] https://sourceforge.net/tracker/?func=detail&aid=2936616&group_id=56967&atid=482468
I saw the tracker item, and was wondering about it.
What did you do to get into the situation? In other words, how can I reproduce?
Unfortunately, I don't have an isolated failing case yet. I am continuously sending queries (mostly SELECT, INSERT and DELETE) using MonetDB-SQL, many of which are grouped inside large transactions. After initializing the database, it takes a while for the issue to occur, and I'm not yet sure what triggers it.
I am currently adding instrumentation code around the code communicating with Mapi, which will hopefully get me some reproducible sessions for this and the other issue I am seeing
I don't think BATs created in VIEWcreate_ should end up in GDKupgradevarheap. Those BATs are views, and I would think views are read-only, and read-only BATs should not end up here.
Yes, intuitively it seemed a bit unusual when I thought about it, but then again I am not much used to the MonetDB internals yet.
I had found GDKupgradevarheap being supplied with a capacity less than heap.size >> shift (it was capacity == 256, heap.size == 512 and shift == 0), and was wondering where it came from. So I added assert statements to all locations modifying capacity, shift or the heap allocation, which errored out in VIEWcreate_. It happened about the same time where I would have expected the GDKupgradevarheap issue to occur, so I found it most likely to be related.
Of course the problematic BAT might still come from another source, so if you say views won't reach GDKupgradevarheap, I'll probably go for another run without the assertions in VIEWcreate_. On the other hand, will it suffice to put assert(!isVIEW(b)) statements in front of the GDKupgradevarheap calls in the code to verify?
If you get to this situation again, and you happen to catch it in the debugger (inside GDKupgradevarheap), perhaps you can look at the c->parentid value. I expect it to be 0, meaning the column is not shared with another BAT (i.e. not a view). I'll try an assert to that effect and see what that gives in our tests.
The BAT capacity is the number of elements that can be stored in the BAT, i.e. it is exactly the size of the heap (apart from the multiplication with the width of the elements). However, this may break down in the case of views. When I wrote the code, I wasn't thinking of views, since I didn't expect them to end up here, and I still think they shouldn't.
Having said this, it is probably possible, perhaps even equivalent, to use the size as you suggested instead of the capacity -- as long as views are left out of the equation.
So, as views aren't supposed to get there anyway, capacity is redundant in that function, right?
I think you're right that the capacity argument is redundant. I have a patch ready for that. It's slightly different from your patch, but the effect is the same. -- Sjoerd Mullender