Changeset: 278c9841f71f for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/278c9841f71f
Modified Files:
gdk/gdk_bat.c
gdk/gdk_logger.c
Branch: Dec2023
Log Message:
Some improvements to cleanup_and_swap and bm_subcommit.
If memory is tight, we want to fail *before* we mess around with the
persistency of bats. Also, we don't have to actually load bats that we
want to make transient (BATmode does not look at the data), so this
eliminates a potential failure.
Before, if loading a bat failed, it would keep a logical reference
forever. And if creating the new catalog bats failed, we would
potentially have a lot of bats that would never be released and might
stay (dirty) in memory forever.
In both cleanup_and_swap and bm_subcommit, we cleanup any bats that are
slated to be deleted, whether they were mentioned in the current
transaction (when flushing a WAL log file) or not.
diffs (164 lines):
diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c
--- a/gdk/gdk_bat.c
+++ b/gdk/gdk_bat.c
@@ -2545,6 +2545,8 @@ BATmode(BAT *b, bool transient)
}
BATiter bi = bat_iterator(b);
+ bool mustrelease = false;
+ bat bid = b->batCacheid;
if (transient != bi.transient) {
bat bid = b->batCacheid;
@@ -2566,7 +2568,13 @@ BATmode(BAT *b, bool transient)
if (!transient) {
BBPretain(bid);
} else if (!bi.transient) {
- BBPrelease(bid);
+ /* we need to delay the release because if there
+ * is no fix and the bat is loaded, BBPrelease
+ * can call BBPfree which calls BATfree which
+ * may hang while waiting for the heap reference
+ * that we have because of the BAT iterator to
+ * come down, in other words, deadlock */
+ mustrelease = true;
}
MT_lock_set(&GDKswapLock(bid));
if (!transient) {
@@ -2597,6 +2605,9 @@ BATmode(BAT *b, bool transient)
MT_lock_unset(&GDKswapLock(bid));
}
bat_iterator_end(&bi);
+ /* release after bat_iterator_end because of refs to heaps */
+ if (mustrelease)
+ BBPrelease(bid);
return GDK_SUCCEED;
}
diff --git a/gdk/gdk_logger.c b/gdk/gdk_logger.c
--- a/gdk/gdk_logger.c
+++ b/gdk/gdk_logger.c
@@ -1533,35 +1533,12 @@ subcommit_list_add(int next, bat *n, BUN
static int
cleanup_and_swap(logger *lg, int *r, const log_bid *bids, lng *lids, lng *cnts,
- BAT *catalog_bid, BAT *catalog_id, BAT *dcatalog, BUN cleanup,
- uint32_t *updated, BUN maxupdated)
+ BAT *catalog_bid, BAT *catalog_id, BAT *dcatalog, BUN cleanup)
{
BAT *nbids, *noids, *ncnts, *nlids, *ndels;
BUN p, q;
int err = 0, rcnt = 0;
- oid *poss = Tloc(dcatalog, 0);
- BATloop(dcatalog, p, q) {
- oid pos = poss[p];
-
- if (updated && pos < maxupdated && (updated[pos / 32] & (1U <<
(pos % 32))) == 0) {
- continue;
- }
- if (lids[pos] == lng_nil || lids[pos] > lg->saved_tid)
- continue;
-
- if (lids[pos] >= 0) {
- lids[pos] = -1; /* mark as transient */
- r[rcnt++] = bids[pos];
-
- BAT *lb;
-
- if ((lb = BATdescriptor(bids[pos])) == NULL ||
BATmode(lb, true /*transient */ ) != GDK_SUCCEED) {
- GDKwarning("Failed to set bat(%d) transient\n",
bids[pos]);
- }
- logbat_destroy(lb);
- }
- }
BUN ocnt = BATcount(catalog_bid);
nbids = logbat_new(TYPE_int, ocnt - cleanup, PERSISTENT);
noids = logbat_new(TYPE_int, ocnt - cleanup, PERSISTENT);
@@ -1578,6 +1555,26 @@ cleanup_and_swap(logger *lg, int *r, con
return 0;
}
+ oid *poss = Tloc(dcatalog, 0);
+ BATloop(dcatalog, p, q) {
+ oid pos = poss[p];
+
+ if (lids[pos] == lng_nil || lids[pos] > lg->saved_tid)
+ continue;
+
+ if (lids[pos] >= 0) {
+ BAT *lb;
+ bat bid = bids[pos];
+
+ if ((lb = BBP_desc(bid)) == NULL || BATmode(lb, true
/*transient */ ) != GDK_SUCCEED) {
+ GDKwarning("Failed to set bat(%d) transient\n",
bid);
+ } else {
+ lids[pos] = -1; /* mark as transient */
+ r[rcnt++] = bid;
+ }
+ }
+ }
+
int *oids = (int *) Tloc(catalog_id, 0);
q = BATcount(catalog_bid);
for (p = 0; p < q && !err; p++) {
@@ -1589,7 +1586,7 @@ cleanup_and_swap(logger *lg, int *r, con
/* only project out the deleted with lid == -1
* update dcatalog */
- if ((updated == NULL || p >= maxupdated || (updated[p / 32] &
(1U << (p % 32))) != 0) && lid == -1)
+ if (lid == -1)
continue; /* remove */
if (BUNappend(nbids, &col, false) != GDK_SUCCEED ||
@@ -1701,11 +1698,6 @@ bm_subcommit(logger *lg, logged_range *p
if (lg->catalog_lid)
lids = (lng *) Tloc(lg->catalog_lid, 0);
BATloop(catalog_bid, p, q) {
- if (updated && p < maxupdated && (updated[p / 32] & (1U << (p %
32))) == 0) {
- continue;
- }
- bat col = bids[p];
-
if (lids && lids[p] != lng_nil && lids[p] <= lg->saved_tid) {
cleanup++;
if (lids[p] == -1)
@@ -1724,6 +1716,11 @@ bm_subcommit(logger *lg, logged_range *p
return GDK_FAIL;
}
}
+ if (updated && p < maxupdated && (updated[p / 32] & (1U << (p %
32))) == 0) {
+ continue;
+ }
+ bat col = bids[p];
+
TRC_DEBUG(WAL, "new %s (%d)\n", BBP_logical(col), col);
assert(col);
sizes[i] = cnts ? (BUN) cnts[p] : 0;
@@ -1740,8 +1737,7 @@ bm_subcommit(logger *lg, logged_range *p
if (cleanup) {
if ((rcnt = cleanup_and_swap(lg, r, bids, lids, cnts,
catalog_bid, catalog_id, dcatalog,
- cleanup, updated,
- maxupdated)) < 0) {
+ cleanup)) < 0) {
GDKfree(n);
GDKfree(r);
GDKfree(sizes);
@@ -3265,15 +3261,13 @@ bm_commit(logger *lg, logged_range *pend
BAT *lb;
assert(bid);
- if ((lb = BATdescriptor(bid)) == NULL || BATmode(lb, false) !=
GDK_SUCCEED) {
+ if ((lb = BBP_desc(bid)) == NULL || BATmode(lb, false) !=
GDK_SUCCEED) {
GDKwarning("Failed to set bat (%d%s) persistent\n",
bid, !lb ? " gone" : "");
- logbat_destroy(lb);
log_unlock(lg);
return GDK_FAIL;
}
assert(lb->batRestricted != BAT_WRITE);
- logbat_destroy(lb);
TRC_DEBUG(WAL, "create %d (%d)\n", bid, BBP_lrefs(bid));
}
_______________________________________________
checkin-list mailing list -- [email protected]
To unsubscribe send an email to [email protected]
