Changeset: 7797ec88f7e1 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/7797ec88f7e1
Modified Files:
gdk/gdk.h
gdk/gdk_bat.c
gdk/gdk_batop.c
gdk/gdk_heap.c
gdk/gdk_string.c
sql/test/emptydb-upgrade-chain/Tests/upgrade.stable.out
sql/test/emptydb-upgrade/Tests/upgrade.stable.out
sql/test/testdb-upgrade-chain/Tests/upgrade.stable.out
sql/test/testdb-upgrade/Tests/upgrade.stable.out
Branch: Dec2023
Log Message:
Merge with Jun2023 branch.
diffs (210 lines):
diff --git a/gdk/ChangeLog.Jun2023 b/gdk/ChangeLog.Jun2023
--- a/gdk/ChangeLog.Jun2023
+++ b/gdk/ChangeLog.Jun2023
@@ -1,3 +1,9 @@
# ChangeLog file for GDK
# This file is updated with Maddlog
+* Tue Nov 21 2023 Sjoerd Mullender <sjo...@acm.org>
+- Fixed a (rare) race condition between copying a bat (COLcopy) and
+ updates happening in parallel to that same bat. This may only be
+ an actual problem with string bats, and then only in very particular
+ circumstances.
+
diff --git a/gdk/gdk.h b/gdk/gdk.h
--- a/gdk/gdk.h
+++ b/gdk/gdk.h
@@ -1742,7 +1742,10 @@ tfastins_nocheckVAR(BAT *b, BUN p, const
gdk_return rc;
assert(b->tbaseoff == 0);
assert(b->theap->parentid == b->batCacheid);
- if ((rc = ATOMputVAR(b, &d, v)) != GDK_SUCCEED)
+ MT_lock_set(&b->theaplock);
+ rc = ATOMputVAR(b, &d, v);
+ MT_lock_unset(&b->theaplock);
+ if (rc != GDK_SUCCEED)
return rc;
if (b->twidth < SIZEOF_VAR_T &&
(b->twidth <= 2 ? d - GDK_VAROFFSET : d) >= ((size_t) 1 << (8 <<
b->tshift))) {
diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c
--- a/gdk/gdk_bat.c
+++ b/gdk/gdk_bat.c
@@ -812,7 +812,8 @@ COLcopy(BAT *b, int tt, bool writable, r
return NULL;
}
- bi = bat_iterator(b);
+ MT_lock_set(&b->theaplock);
+ bi = bat_iterator_nolock(b);
/* first try case (1); create a view, possibly with different
* atom-types */
@@ -822,9 +823,9 @@ COLcopy(BAT *b, int tt, bool writable, r
ATOMstorage(b->ttype) != TYPE_msk && /* no view on TYPE_msk */
(!VIEWtparent(b) ||
BBP_desc(VIEWtparent(b))->batRestricted == BAT_READ)) {
+ MT_lock_unset(&b->theaplock);
bn = VIEWcreate(b->hseqbase, b);
if (bn == NULL) {
- bat_iterator_end(&bi);
return NULL;
}
if (tt != bn->ttype) {
@@ -837,6 +838,7 @@ COLcopy(BAT *b, int tt, bool writable, r
}
bn->tseqbase = ATOMtype(tt) == TYPE_oid ? bi.tseq :
oid_nil;
}
+ return bn;
} else {
/* check whether we need case (4); BUN-by-BUN copy (by
* setting slowcopy to false) */
@@ -863,7 +865,7 @@ COLcopy(BAT *b, int tt, bool writable, r
bn = COLnew2(b->hseqbase, tt, bi.count, role, bi.width);
if (bn == NULL) {
- bat_iterator_end(&bi);
+ MT_lock_unset(&b->theaplock);
return NULL;
}
if (bn->tvheap != NULL && bn->tvheap->base == NULL) {
@@ -1006,10 +1008,10 @@ COLcopy(BAT *b, int tt, bool writable, r
bn->batRestricted = BAT_READ;
TRC_DEBUG(ALGO, ALGOBATFMT " -> " ALGOBATFMT "\n",
ALGOBATPAR(b), ALGOBATPAR(bn));
- bat_iterator_end(&bi);
+ MT_lock_unset(&b->theaplock);
return bn;
bunins_failed:
- bat_iterator_end(&bi);
+ MT_lock_unset(&b->theaplock);
BBPreclaim(bn);
return NULL;
}
diff --git a/gdk/gdk_batop.c b/gdk/gdk_batop.c
--- a/gdk/gdk_batop.c
+++ b/gdk/gdk_batop.c
@@ -1392,7 +1392,10 @@ BATappend_or_update(BAT *b, BAT *p, cons
default:
MT_UNREACHABLE();
}
- if (ATOMreplaceVAR(b, &d, new) != GDK_SUCCEED) {
+ MT_lock_set(&b->theaplock);
+ gdk_return rc = ATOMreplaceVAR(b, &d, new);
+ MT_lock_unset(&b->theaplock);
+ if (rc != GDK_SUCCEED) {
goto bailout;
}
if (b->twidth < SIZEOF_VAR_T &&
diff --git a/gdk/gdk_heap.c b/gdk/gdk_heap.c
--- a/gdk/gdk_heap.c
+++ b/gdk/gdk_heap.c
@@ -1185,15 +1185,12 @@ HEAP_malloc(BAT *b, size_t nbytes)
/* Increase the size of the heap. */
TRC_DEBUG(HEAP, "HEAPextend in HEAP_malloc %s %zu %zu\n",
heap->filename, heap->size, newsize);
- MT_lock_set(&b->theaplock);
if (HEAPgrow(&b->tvheap, newsize, false) != GDK_SUCCEED) {
- MT_lock_unset(&b->theaplock);
return (var_t) -1;
}
heap = b->tvheap;
heap->free = newsize;
heap->dirty = true;
- MT_lock_unset(&b->theaplock);
hheader = HEAP_index(heap, 0, HEADER);
blockp = HEAP_index(heap, block, CHUNK);
diff --git a/gdk/gdk_string.c b/gdk/gdk_string.c
--- a/gdk/gdk_string.c
+++ b/gdk/gdk_string.c
@@ -190,17 +190,14 @@ strPut(BAT *b, var_t *dst, const void *V
BUN off;
if (h->free == 0) {
- MT_lock_set(&b->theaplock);
if (h->size < GDK_STRHASHTABLE * sizeof(stridx_t) + BATTINY *
GDK_VARALIGN) {
if (HEAPgrow(&b->tvheap, GDK_STRHASHTABLE *
sizeof(stridx_t) + BATTINY * GDK_VARALIGN, true) != GDK_SUCCEED) {
- MT_lock_unset(&b->theaplock);
return (var_t) -1;
}
h = b->tvheap;
}
h->free = GDK_STRHASHTABLE * sizeof(stridx_t);
h->dirty = true;
- MT_lock_unset(&b->theaplock);
#ifdef NDEBUG
memset(h->base, 0, h->free);
#else
@@ -286,13 +283,10 @@ strPut(BAT *b, var_t *dst, const void *V
return (var_t) -1;
}
TRC_DEBUG(HEAP, "HEAPextend in strPut %s %zu %zu\n",
h->filename, h->size, newsize);
- MT_lock_set(&b->theaplock);
if (HEAPgrow(&b->tvheap, newsize, true) != GDK_SUCCEED) {
- MT_lock_unset(&b->theaplock);
return (var_t) -1;
}
h = b->tvheap;
- MT_lock_unset(&b->theaplock);
/* make bucket point into the new heap */
bucket = ((stridx_t *) h->base) + off;
@@ -304,10 +298,8 @@ strPut(BAT *b, var_t *dst, const void *V
if (pad > 0)
memset(h->base + h->free, 0, pad);
memcpy(h->base + pos, v, len);
- MT_lock_set(&b->theaplock);
h->free += pad + len;
h->dirty = true;
- MT_lock_unset(&b->theaplock);
/* maintain hash table */
if (GDK_ELIMBASE(pos) == 0) { /* small string heap: link the next
pointer */
diff --git a/sql/test/emptydb-upgrade-chain/Tests/upgrade.stable.out
b/sql/test/emptydb-upgrade-chain/Tests/upgrade.stable.out
--- a/sql/test/emptydb-upgrade-chain/Tests/upgrade.stable.out
+++ b/sql/test/emptydb-upgrade-chain/Tests/upgrade.stable.out
@@ -550,8 +550,3 @@ GRANT EXECUTE ON FUNCTION sys.persist_un
UPDATE sys.functions SET system = true WHERE system <> true AND
name = 'persist_unlogged' AND schema_id = 2000;
-Running database upgrade commands:
-CREATE FUNCTION timestamp_to_str(d TIMESTAMP, format STRING) RETURNS STRING
EXTERNAL NAME mtime."timestamp_to_str";
-GRANT EXECUTE ON FUNCTION timestamp_to_str(TIMESTAMP, STRING) TO PUBLIC;
-UPDATE sys.functions SET system = true WHERE system <> true AND name =
'timestamp_to_str' AND schema_id = 2000 and type = 1;
-
diff --git a/sql/test/emptydb-upgrade/Tests/upgrade.stable.out
b/sql/test/emptydb-upgrade/Tests/upgrade.stable.out
--- a/sql/test/emptydb-upgrade/Tests/upgrade.stable.out
+++ b/sql/test/emptydb-upgrade/Tests/upgrade.stable.out
@@ -550,8 +550,3 @@ GRANT EXECUTE ON FUNCTION sys.persist_un
UPDATE sys.functions SET system = true WHERE system <> true AND
name = 'persist_unlogged' AND schema_id = 2000;
-Running database upgrade commands:
-CREATE FUNCTION timestamp_to_str(d TIMESTAMP, format STRING) RETURNS STRING
EXTERNAL NAME mtime."timestamp_to_str";
-GRANT EXECUTE ON FUNCTION timestamp_to_str(TIMESTAMP, STRING) TO PUBLIC;
-UPDATE sys.functions SET system = true WHERE system <> true AND name =
'timestamp_to_str' AND schema_id = 2000 and type = 1;
-
diff --git a/sql/test/testdb-upgrade-chain/Tests/upgrade.stable.out
b/sql/test/testdb-upgrade-chain/Tests/upgrade.stable.out
--- a/sql/test/testdb-upgrade-chain/Tests/upgrade.stable.out
+++ b/sql/test/testdb-upgrade-chain/Tests/upgrade.stable.out
@@ -550,8 +550,3 @@ GRANT EXECUTE ON FUNCTION sys.persist_un
UPDATE sys.functions SET system = true WHERE system <> true AND
name = 'persist_unlogged' AND schema_id = 2000;
-Running database upgrade commands:
-CREATE FUNCTION timestamp_to_str(d TIMESTAMP, format STRING) RETURNS STRING
EXTERNAL NAME mtime."timestamp_to_str";
-GRANT EXECUTE ON FUNCTION timestamp_to_str(TIMESTAMP, STRING) TO PUBLIC;
-UPDATE sys.functions SET system = true WHERE system <> true AND name =
'timestamp_to_str' AND schema_id = 2000 and type = 1;
-
diff --git a/sql/test/testdb-upgrade/Tests/upgrade.stable.out
b/sql/test/testdb-upgrade/Tests/upgrade.stable.out
--- a/sql/test/testdb-upgrade/Tests/upgrade.stable.out
+++ b/sql/test/testdb-upgrade/Tests/upgrade.stable.out
@@ -550,8 +550,3 @@ GRANT EXECUTE ON FUNCTION sys.persist_un
UPDATE sys.functions SET system = true WHERE system <> true AND
name = 'persist_unlogged' AND schema_id = 2000;
-Running database upgrade commands:
-CREATE FUNCTION timestamp_to_str(d TIMESTAMP, format STRING) RETURNS STRING
EXTERNAL NAME mtime."timestamp_to_str";
-GRANT EXECUTE ON FUNCTION timestamp_to_str(TIMESTAMP, STRING) TO PUBLIC;
-UPDATE sys.functions SET system = true WHERE system <> true AND name =
'timestamp_to_str' AND schema_id = 2000 and type = 1;
-
_______________________________________________
checkin-list mailing list -- checkin-list@monetdb.org
To unsubscribe send an email to checkin-list-le...@monetdb.org
