Changeset: 8adfdd8687f8 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/8adfdd8687f8
Modified Files:
gdk/gdk_bbp.c
gdk/gdk_private.h
gdk/gdk_storage.c
Branch: Jul2021
Log Message:
Create BBP.dir file during save stage with bat locks in place.
This should prevent the race condition where a bat was first saved,
and then, in a separate action, written to the BBP.dir file, giving
other threads the opportunity to change the bat between those actions.
diffs (truncated from 429 to 300 lines):
diff --git a/gdk/gdk_bbp.c b/gdk/gdk_bbp.c
--- a/gdk/gdk_bbp.c
+++ b/gdk/gdk_bbp.c
@@ -114,7 +114,7 @@ static void BBPuncacheit(bat bid, bool u
static gdk_return BBPprepare(bool subcommit);
static BAT *getBBPdescriptor(bat i, bool lock);
static gdk_return BBPbackup(BAT *b, bool subcommit);
-static gdk_return BBPdir(int cnt, bat *restrict subcommit, BUN *restrict
sizes, lng logno, lng transid);
+static gdk_return BBPdir_init(void);
static lng BBPlogno; /* two lngs of extra info in BBP.dir */
static lng BBPtransid;
@@ -1109,7 +1109,7 @@ BBPinit(void)
/* no BBP.bak (nor BBP.dir or BACKUP/BBP.dir):
* create a new one */
TRC_DEBUG(IO_, "initializing BBP.\n");
- if (BBPdir(0, NULL, NULL, LL_CONSTANT(0),
LL_CONSTANT(0)) != GDK_SUCCEED) {
+ if (BBPdir_init() != GDK_SUCCEED) {
GDKfree(bbpdirstr);
GDKfree(backupbbpdirstr);
MT_lock_unset(&GDKtmLock);
@@ -1401,99 +1401,146 @@ BBPdir_header(FILE *f, int n, lng logno,
}
static gdk_return
-BBPdir_subcommit(int cnt, bat *restrict subcommit, BUN *restrict sizes, lng
logno, lng transid)
+BBPdir_first(bool subcommit, lng logno, lng transid,
+ FILE **obbpfp, FILE **nbbpfp)
{
- FILE *obbpf, *nbbpf;
- bat j = 1;
- char buf[3000];
- int n;
+ FILE *obbpf = NULL, *nbbpf = NULL;
+ int n = 0;
lng ologno, otransid;
#ifndef NDEBUG
assert(islocked(&GDKtmLock));
- assert(subcommit != NULL);
- for (n = 2; n < cnt; n++)
- assert(subcommit[n - 1] < subcommit[n]);
#endif
- if ((nbbpf = GDKfilelocate(0, "BBP", "w", "dir")) == NULL)
+ if (obbpfp)
+ *obbpfp = NULL;
+ *nbbpfp = NULL;
+
+ if ((nbbpf = GDKfilelocate(0, "BBP", "w", "dir")) == NULL) {
return GDK_FAIL;
-
- /* we need to copy the backup BBP.dir to the new, but
- * replacing the entries for the subcommitted bats */
- if ((obbpf = GDKfileopen(0, SUBDIR, "BBP", "dir", "r")) == NULL &&
- (obbpf = GDKfileopen(0, BAKDIR, "BBP", "dir", "r")) == NULL) {
- GDKsyserror("subcommit attempted without backup BBP.dir.");
- goto bailout;
}
- /* read first three lines */
- if (fgets(buf, sizeof(buf), obbpf) == NULL || /* BBP.dir, GDKversion %d
*/
- fgets(buf, sizeof(buf), obbpf) == NULL || /* SIZEOF_SIZE_T
SIZEOF_OID SIZEOF_MAX_INT */
- fgets(buf, sizeof(buf), obbpf) == NULL) { /* BBPsize=%d */
- GDKerror("subcommit attempted with invalid backup BBP.dir.");
- goto bailout;
+
+ if (subcommit) {
+ char buf[512];
+
+ assert(obbpfp != NULL);
+ /* we need to copy the backup BBP.dir to the new, but
+ * replacing the entries for the subcommitted bats */
+ if ((obbpf = GDKfileopen(0, SUBDIR, "BBP", "dir", "r")) == NULL
&&
+ (obbpf = GDKfileopen(0, BAKDIR, "BBP", "dir", "r")) ==
NULL) {
+ GDKsyserror("subcommit attempted without backup
BBP.dir.");
+ goto bailout;
+ }
+ /* read first three lines */
+ if (fgets(buf, sizeof(buf), obbpf) == NULL || /* BBP.dir,
GDKversion %d */
+ fgets(buf, sizeof(buf), obbpf) == NULL || /* SIZEOF_SIZE_T
SIZEOF_OID SIZEOF_MAX_INT */
+ fgets(buf, sizeof(buf), obbpf) == NULL) { /* BBPsize=%d */
+ GDKerror("subcommit attempted with invalid backup
BBP.dir.");
+ goto bailout;
+ }
+ /* third line contains BBPsize */
+ if (sscanf(buf, "BBPsize=%d", &n) != 1) {
+ GDKerror("cannot read BBPsize in backup BBP.dir.");
+ goto bailout;
+ }
+ /* fourth line contains BBPinfo */
+ if (fgets(buf, sizeof(buf), obbpf) == NULL ||
+ sscanf(buf, "BBPinfo=" LLSCN " " LLSCN, &ologno, &otransid)
!= 2) {
+ GDKerror("cannot read BBPinfo in backup BBP.dir.");
+ goto bailout;
+ }
}
- /* third line contains BBPsize */
- if (sscanf(buf, "BBPsize=%d", &n) != 1) {
- GDKerror("cannot read BBPsize in backup BBP.dir.");
- goto bailout;
- }
+
if (n < (bat) ATOMIC_GET(&BBPsize))
n = (bat) ATOMIC_GET(&BBPsize);
- /* fourth line contains BBPinfo */
- if (fgets(buf, sizeof(buf), obbpf) == NULL ||
- sscanf(buf, "BBPinfo=" LLSCN " " LLSCN, &ologno, &otransid) != 2) {
- GDKerror("cannot read BBPinfo in backup BBP.dir.");
- goto bailout;
- }
TRC_DEBUG(IO_, "writing BBP.dir (%d bats).\n", n);
if (BBPdir_header(nbbpf, n, logno, transid) != GDK_SUCCEED) {
goto bailout;
}
- n = 0;
- for (;;) {
- /* but for subcommits, all except the bats in the list
- * retain their existing mode */
- if (n == 0 && obbpf != NULL) {
- if (fgets(buf, sizeof(buf), obbpf) == NULL) {
- fclose(obbpf);
- obbpf = NULL;
- } else if (sscanf(buf, "%d", &n) != 1 || n <= 0) {
+
+ if (obbpfp)
+ *obbpfp = obbpf;
+ *nbbpfp = nbbpf;
+
+ return GDK_SUCCEED;
+
+ bailout:
+ if (obbpf != NULL)
+ fclose(obbpf);
+ if (nbbpf != NULL)
+ fclose(nbbpf);
+ return GDK_FAIL;
+}
+
+static bat
+BBPdir_step(bat bid, BUN size, int n, char *buf, size_t bufsize,
+ FILE **obbpfp, FILE *nbbpf)
+{
+ if (n < -1) /* safety catch */
+ return n;
+ while (n >= 0 && n < bid) {
+ if (n > 0 && fputs(buf, nbbpf) == EOF) {
+ GDKerror("Writing BBP.dir file failed.\n");
+ goto bailout;
+ }
+ if (fgets(buf, bufsize, *obbpfp) == NULL) {
+ if (ferror(*obbpfp)) {
+ GDKerror("error reading backup BBP.dir.");
+ goto bailout;
+ }
+ n = -1;
+ if (fclose(*obbpfp) == EOF) {
+ GDKsyserror("Closing backup BBP.dir file
failed.\n");
+ GDKclrerr(); /* ignore error */
+ }
+ *obbpfp = NULL;
+ } else {
+ if (sscanf(buf, "%d", &n) != 1 || n <= 0) {
GDKerror("subcommit attempted with invalid
backup BBP.dir.");
goto bailout;
}
- /* at this point, obbpf == NULL, or n > 0 */
}
- if (j == cnt && n == 0) {
- assert(obbpf == NULL);
- break;
- }
- if (j < cnt && (n == 0 || subcommit[j] <= n || obbpf == NULL)) {
- bat i = subcommit[j];
- /* BBP.dir consists of all persistent bats only */
- if (BBP_status(i) & BBPPERSISTENT) {
- if (new_bbpentry(nbbpf, i, sizes ? sizes[j] :
BUN_NONE) != GDK_SUCCEED) {
- goto bailout;
- }
- }
- if (i == n)
- n = 0; /* read new entry (i.e. skip this one
from old BBP.dir */
- do
- /* go to next, skipping duplicates */
- j++;
- while (j < cnt && subcommit[j] == i);
- } else {
- if (fprintf(nbbpf, "%s", buf) < 0) {
- GDKsyserror("Copying BBP.dir entry failed\n");
+ }
+ if (BBP_status(bid) & BBPPERSISTENT) {
+ if (new_bbpentry(nbbpf, bid, size) != GDK_SUCCEED)
+ goto bailout;
+ }
+ return n == -1 ? -1 : n == bid ? 0 : n;
+
+ bailout:
+ if (*obbpfp)
+ fclose(*obbpfp);
+ fclose(nbbpf);
+ return -2;
+}
+
+static gdk_return
+BBPdir_last(int n, char *buf, size_t bufsize, FILE *obbpf, FILE *nbbpf)
+{
+ if (n > 0 && fputs(buf, nbbpf) == EOF) {
+ GDKerror("Writing BBP.dir file failed.\n");
+ goto bailout;
+ }
+ while (obbpf) {
+ if (fgets(buf, bufsize, obbpf) == NULL) {
+ if (ferror(obbpf)) {
+ GDKerror("error reading backup BBP.dir.");
goto bailout;
}
- TRC_DEBUG(IO_, "%s", buf);
- n = 0;
+ if (fclose(obbpf) == EOF) {
+ GDKsyserror("Closing backup BBP.dir file
failed.\n");
+ GDKclrerr(); /* ignore error */
+ }
+ obbpf = NULL;
+ } else {
+ if (fputs(buf, nbbpf) == EOF) {
+ GDKerror("Writing BBP.dir file failed.\n");
+ goto bailout;
+ }
}
}
-
if (fflush(nbbpf) == EOF ||
(!(GDKdebug & NOSYNCMASK)
#if defined(NATIVE_WIN32)
@@ -1525,62 +1572,15 @@ BBPdir_subcommit(int cnt, bat *restrict
}
gdk_return
-BBPdir(int cnt, bat *restrict subcommit, BUN *restrict sizes, lng logno, lng
transid)
+BBPdir_init(void)
{
FILE *fp;
- bat i;
-
- if (subcommit)
- return BBPdir_subcommit(cnt, subcommit, sizes, logno, transid);
-
- TRC_DEBUG(IO_, "writing BBP.dir (%d bats).\n", (int) (bat)
ATOMIC_GET(&BBPsize));
- if ((fp = GDKfilelocate(0, "BBP", "w", "dir")) == NULL) {
- goto bailout;
- }
-
- if (BBPdir_header(fp, (bat) ATOMIC_GET(&BBPsize), logno, transid) !=
GDK_SUCCEED) {
- goto bailout;
- }
-
- for (i = 1; i < (bat) ATOMIC_GET(&BBPsize); i++) {
- /* write the entry
- * BBP.dir consists of all persistent bats */
- if (BBP_status(i) & BBPPERSISTENT) {
- if (new_bbpentry(fp, i, BUN_NONE) != GDK_SUCCEED) {
- goto bailout;
- }
- }
- }
-
- if (fflush(fp) == EOF ||
- (!(GDKdebug & NOSYNCMASK)
-#if defined(NATIVE_WIN32)
- && _commit(_fileno(fp)) < 0
-#elif defined(HAVE_FDATASYNC)
- && fdatasync(fileno(fp)) < 0
-#elif defined(HAVE_FSYNC)
- && fsync(fileno(fp)) < 0
-#endif
- )) {
- GDKsyserror("Syncing BBP.dir file failed\n");
- goto bailout;
- }
- if (fclose(fp) == EOF) {
- GDKsyserror("Closing BBP.dir file failed\n");
- return GDK_FAIL;
- }
-
- TRC_DEBUG(IO_, "end\n");
-
- if (i < (bat) ATOMIC_GET(&BBPsize))
- return GDK_FAIL;
-
- return GDK_SUCCEED;
-
- bailout:
- if (fp != NULL)
- fclose(fp);
- return GDK_FAIL;
+ gdk_return rc;
+
+ rc = BBPdir_first(false, 0, 0, NULL, &fp);
+ if (rc == GDK_SUCCEED)
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list
