From cb72e314ef0285a5a4d2734b0715bfb0d450f6b5 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 9 Apr 2019 22:06:48 +0000 Subject: [PATCH] storage: don't crash on new seqno error from rocks ingest If rocks has already compacted our file away, the link count might not be >1 but it could still reject repeated ingestion. We can just fall back to the copy, and any real error will be surfaced when we try to ingest it. Release note: none. --- pkg/storage/replica_proposal.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/storage/replica_proposal.go b/pkg/storage/replica_proposal.go index 6cd358e55e05..e47cc74d16cf 100644 --- a/pkg/storage/replica_proposal.go +++ b/pkg/storage/replica_proposal.go @@ -444,8 +444,18 @@ func addSSTablePreApply( log.Fatalf(ctx, "failed to move ingest sst: %v", rmErr) } const seqNoMsg = "Global seqno is required, but disabled" - if err, ok := ingestErr.(*engine.RocksDBError); ok && !strings.Contains(ingestErr.Error(), seqNoMsg) { - log.Fatalf(ctx, "while ingesting %s: %s", ingestPath, err) + const seqNoOnReIngest = "external file have non zero sequence number" + // Repeated ingestion is still possible even with the link count checked + // above, since rocks might have already compacted away the file. + // However it does not flush compacted files from its cache, so it can + // still react poorly to attempting to ingest again. If we get an error + // that indicates we can't ingest, we'll make a copy and try again. That + // attempt must succeed or we'll fatal, so any persistent error is still + // going to be surfaced. + ingestErrMsg := ingestErr.Error() + isSeqNoErr := strings.Contains(ingestErrMsg, seqNoMsg) || strings.Contains(ingestErrMsg, seqNoOnReIngest) + if _, ok := ingestErr.(*engine.RocksDBError); !ok || !isSeqNoErr { + log.Fatalf(ctx, "while ingesting %s: %s", ingestPath, ingestErr) } } }