db: tolerate unknown options

Instead of throwing an error when we encounter unfamiliar options, we
will instead tolerate it and log a message. This will allow us to be
more lenient about options handling (backporting, renaming, etc.). It
also addresses the case in which a user could revert before a major
upgrade finalization and have some new option written that the lesser
version doesn't recognize and error out at.

Author: annrpom

metamorphic/options.go

@@ -66,47 +66,37 @@ func parseOptions(
 			}
 			return pebble.KeySchema{}, errors.Newf("unknown key schema %q", name)
 		},
-		SkipUnknown: func(name, value string) bool {
+		OnUnknown: func(name, value string) {
 			if strings.EqualFold(value, "false") {
 				// TODO(radu): audit all settings and use ParseBool wherever necessary.
 				panic(fmt.Sprintf("%s: boolean options can only be set to true", name))
 			}
 			switch name {
 			case "TestOptions":
-				return true
 			case "TestOptions.strictfs":
 				opts.strictFS = true
 				opts.Opts.FS = vfs.NewCrashableMem()
-				return true
 			case "TestOptions.key_format":
 				opts.KeyFormat = keyFormatsByName[value]
-				return true
 			case "TestOptions.ingest_using_apply":
 				opts.ingestUsingApply = true
-				return true
 			case "TestOptions.delete_sized":
 				opts.deleteSized = true
-				return true
 			case "TestOptions.replace_single_delete":
 				opts.replaceSingleDelete = true
-				return true
 			case "TestOptions.use_disk":
 				opts.useDisk = true
 				opts.Opts.FS = vfs.Default
-				return true
 			case "TestOptions.initial_state_desc":
 				opts.initialStateDesc = value
-				return true
 			case "TestOptions.initial_state_path":
 				opts.initialStatePath = value
-				return true
 			case "TestOptions.threads":
 				v, err := strconv.Atoi(value)
 				if err != nil {
 					panic(err)
 				}
 				opts.Threads = v
-				return true
 			case "TestOptions.disable_block_property_collector":
 				v, err := strconv.ParseBool(value)
 				if err != nil {
@@ -116,99 +106,81 @@ func parseOptions(
 				if v {
 					opts.Opts.BlockPropertyCollectors = nil
 				}
-				return true
 			case "TestOptions.enable_value_blocks":
 				opts.enableValueBlocks = true
 				opts.Opts.Experimental.EnableValueBlocks = func() bool { return true }
-				return true
 			case "TestOptions.disable_value_blocks_for_ingest_sstables":
 				opts.disableValueBlocksForIngestSSTables = true
-				return true
 			case "TestOptions.async_apply_to_db":
 				opts.asyncApplyToDB = true
-				return true
 			case "TestOptions.shared_storage_enabled":
 				opts.sharedStorageEnabled = true
 				if opts.Opts.Experimental.CreateOnShared == remote.CreateOnSharedNone {
 					opts.Opts.Experimental.CreateOnShared = remote.CreateOnSharedAll
 				}
-				return true
 			case "TestOptions.external_storage_enabled":
 				opts.externalStorageEnabled = true
-				return true
 			case "TestOptions.secondary_cache_enabled":
 				opts.secondaryCacheEnabled = true
 				opts.Opts.Experimental.SecondaryCacheSizeBytes = 1024 * 1024 * 32 // 32 MBs
-				return true
 			case "TestOptions.seed_efos":
 				v, err := strconv.ParseUint(value, 10, 64)
 				if err != nil {
 					panic(err)
 				}
 				opts.seedEFOS = v
-				return true
 			case "TestOptions.io_latency_mean":
 				v, err := time.ParseDuration(value)
 				if err != nil {
 					panic(err)
 				}
 				opts.ioLatencyMean = v
-				return true
 			case "TestOptions.io_latency_probability":
 				v, err := strconv.ParseFloat(value, 64)
 				if err != nil {
 					panic(err)
 				}
 				opts.ioLatencyProbability = v
-				return true
 			case "TestOptions.io_latency_seed":
 				v, err := strconv.ParseInt(value, 10, 64)
 				if err != nil {
 					panic(err)
 				}
 				opts.ioLatencySeed = v
-				return true
 			case "TestOptions.ingest_split":
 				// TODO(radu): this should be on by default.
 				opts.ingestSplit = true
 				opts.Opts.Experimental.IngestSplit = func() bool {
 					return true
 				}
-				return true
 			case "TestOptions.use_shared_replicate":
 				opts.useSharedReplicate = true
-				return true
 			case "TestOptions.use_external_replicate":
 				opts.useExternalReplicate = true
-				return true
 			case "TestOptions.use_excise":
 				// TODO(radu): this should be on by default.
 				opts.useExcise = true
-				return true
 			case "TestOptions.use_delete_only_compaction_excises":
 				opts.useDeleteOnlyCompactionExcises = true
 				opts.Opts.Experimental.EnableDeleteOnlyCompactionExcises = func() bool {
 					return opts.useDeleteOnlyCompactionExcises
 				}
-				return true
 			case "TestOptions.disable_downloads":
 				opts.disableDownloads = true
-				return true
 			case "TestOptions.use_jemalloc_size_classes":
 				opts.Opts.AllocatorSizeClasses = pebble.JemallocSizeClasses
-				return true
 			default:
 				if customOptionParsers == nil {
-					return false
+					return
 				}
 				name = strings.TrimPrefix(name, "TestOptions.")
 				if p, ok := customOptionParsers[name]; ok {
 					if customOpt, ok := p(value); ok {
 						opts.CustomOpts = append(opts.CustomOpts, customOpt)
-						return true
+						return
 					}
 				}
-				return false
+				return
 			}
 		},
 	}

options.go

@@ -1960,7 +1960,7 @@ type ParseHooks struct {
 	NewFilterPolicy func(name string) (FilterPolicy, error)
 	NewKeySchema    func(name string) (KeySchema, error)
 	NewMerger       func(name string) (*Merger, error)
-	SkipUnknown     func(name, value string) bool
+	OnUnknown       func(name, value string)
 }
 
 // Parse parses the options from the specified string. Note that certain
@@ -1976,11 +1976,6 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 	}
 
 	visitKeyValue := func(i, j int, section, key, value string) error {
-		// WARNING: DO NOT remove entries from the switches below because doing so
-		// causes a key previously written to the OPTIONS file to be considered unknown,
-		// a backwards incompatible change. Instead, leave in support for parsing the
-		// key but simply don't parse the value.
-
 		parseComparer := func(name string) (*Comparer, error) {
 			switch name {
 			case DefaultComparer.Name:
@@ -2000,11 +1995,15 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 			switch key {
 			case "pebble_version":
 			default:
-				if hooks != nil && hooks.SkipUnknown != nil && hooks.SkipUnknown(section+"."+key, value) {
+				if hooks != nil && hooks.OnUnknown != nil {
+					hooks.OnUnknown(section+"."+key, value)
 					return nil
 				}
-				return errors.Errorf("pebble: unknown option: %s.%s",
-					errors.Safe(section), errors.Safe(key))
+				// Tolerate unknown options, but log them.
+				if o.Logger != nil {
+					o.Logger.Infof("pebble: unknown option: %s.%s", errors.Safe(section), errors.Safe(key))
+				}
+				return nil
 			}
 			return nil
 
@@ -2059,8 +2058,6 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 				o.private.disableLazyCombinedIteration, err = strconv.ParseBool(value)
 			case "disable_wal":
 				o.DisableWAL, err = strconv.ParseBool(value)
-			case "enable_columnar_blocks":
-				// Do nothing; option existed in older versions of pebble.
 			case "flush_delay_delete_range":
 				o.FlushDelayDeleteRange, err = time.ParseDuration(value)
 			case "flush_delay_range_key":
@@ -2074,7 +2071,18 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 				// version is valid right here.
 				var v uint64
 				v, err = strconv.ParseUint(value, 10, 64)
-				if vers := FormatMajorVersion(v); vers > internalFormatNewest || vers == FormatDefault {
+				vers := FormatMajorVersion(v)
+				if vers > internalFormatNewest {
+					// Tolerate new unknown format versions in OPTIONS file (they
+					// may be stale if a format upgrade was reverted before
+					// finalization). The actual format version is determined by
+					// the format version marker file.
+					if o.Logger != nil {
+						o.Logger.Infof(
+							"pebble: format major version %d in OPTIONS file is newer than supported (%d), ignoring",
+							vers, internalFormatNewest)
+					}
+				} else if vers == FormatDefault {
 					err = errors.Newf("unsupported format major version %d", o.FormatMajorVersion)
 				}
 				if err == nil {
@@ -2118,8 +2126,6 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 				o.L0CompactionThreshold, err = strconv.Atoi(value)
 			case "l0_stop_writes_threshold":
 				o.L0StopWritesThreshold, err = strconv.Atoi(value)
-			case "l0_sublevel_compactions":
-				// Do nothing; option existed in older versions of pebble.
 			case "lbase_max_bytes":
 				o.LBaseMaxBytes, err = strconv.ParseInt(value, 10, 64)
 			case "level_multiplier":
@@ -2146,9 +2152,6 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 				o.MemTableSize, err = strconv.ParseUint(value, 10, 64)
 			case "mem_table_stop_writes_threshold":
 				o.MemTableStopWritesThreshold, err = strconv.Atoi(value)
-			case "min_compaction_rate":
-				// Do nothing; option existed in older versions of pebble, and
-				// may be meaningful again eventually.
 			case "min_deletion_rate":
 				var rate uint64
 				rate, err = strconv.ParseUint(value, 10, 64)
@@ -2159,13 +2162,8 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 				o.DeletionPacing.FreeSpaceThresholdBytes, err = strconv.ParseUint(value, 10, 64)
 			case "free_space_timeframe":
 				o.DeletionPacing.FreeSpaceTimeframe, err = time.ParseDuration(value)
-			case "obsolete_bytes_max_ratio":
-				// No longer used.
 			case "obsolete_bytes_timeframe":
 				o.DeletionPacing.BacklogTimeframe, err = time.ParseDuration(value)
-			case "min_flush_rate":
-				// Do nothing; option existed in older versions of pebble, and
-				// may be meaningful again eventually.
 			case "multilevel_compaction_heuristic":
 				switch {
 				case value == "none":
@@ -2196,10 +2194,11 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 						err = errors.Wrapf(err, "unexpected wamp heuristic arguments: %s", value)
 					}
 				default:
-					err = errors.Newf("unrecognized multilevel compaction heuristic: %s", value)
+					// Tolerate unknown options, but log them.
+					if o.Logger != nil {
+						o.Logger.Infof("pebble: unrecognized multilevel compaction heuristic: %s", value)
+					}
 				}
-			case "point_tombstone_weight":
-				// Do nothing; deprecated.
 			case "strict_wal_tail":
 				var strictWALTail bool
 				strictWALTail, err = strconv.ParseBool(value)
@@ -2240,20 +2239,18 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 				case "leveldb":
 				case "rocksdbv2":
 				default:
-					return errors.Errorf("pebble: unknown table format: %q", errors.Safe(value))
+					// Tolerate unknown options, but log them.
+					if o.Logger != nil {
+						o.Logger.Infof("pebble: unknown table format: %q", errors.Safe(value))
+					}
+					return nil
 				}
-			case "table_property_collectors":
-				// No longer implemented; ignore.
 			case "validate_on_ingest":
 				o.Experimental.ValidateOnIngest, err = strconv.ParseBool(value)
 			case "wal_dir":
 				o.WALDir = value
 			case "wal_bytes_per_sync":
 				o.WALBytesPerSync, err = strconv.Atoi(value)
-			case "max_writer_concurrency":
-				// No longer implemented; ignore.
-			case "force_writer_parallelism":
-				// No longer implemented; ignore.
 			case "secondary_cache_size_bytes":
 				o.Experimental.SecondaryCacheSizeBytes, err = strconv.ParseInt(value, 10, 64)
 			case "create_on_shared":
@@ -2265,11 +2262,15 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 			case "iterator_tracking_max_age":
 				o.Experimental.IteratorTracking.MaxAge, err = time.ParseDuration(value)
 			default:
-				if hooks != nil && hooks.SkipUnknown != nil && hooks.SkipUnknown(section+"."+key, value) {
+				if hooks != nil && hooks.OnUnknown != nil {
+					hooks.OnUnknown(section+"."+key, value)
 					return nil
 				}
-				return errors.Errorf("pebble: unknown option: %s.%s",
-					errors.Safe(section), errors.Safe(key))
+				// Tolerate unknown options, but log them.
+				if o.Logger != nil {
+					o.Logger.Infof("pebble: unknown option: %s.%s", errors.Safe(section), errors.Safe(key))
+				}
+				return nil
 			}
 			return err
 
@@ -2292,10 +2293,15 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 			case "garbage_ratio_high_priority":
 				valSepPolicy.GarbageRatioHighPriority, err = strconv.ParseFloat(value, 64)
 			default:
-				if hooks != nil && hooks.SkipUnknown != nil && hooks.SkipUnknown(section+"."+key, value) {
+				if hooks != nil && hooks.OnUnknown != nil {
+					hooks.OnUnknown(section+"."+key, value)
 					return nil
 				}
-				return errors.Errorf("pebble: unknown option: %s.%s", errors.Safe(section), errors.Safe(key))
+				// Tolerate unknown options, but log them.
+				if o.Logger != nil {
+					o.Logger.Infof("pebble: unknown option: %s.%s", errors.Safe(section), errors.Safe(key))
+				}
+				return nil
 			}
 			return err
 
@@ -2324,11 +2330,15 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 			case "elevated_write_stall_threshold_lag":
 				o.WALFailover.ElevatedWriteStallThresholdLag, err = time.ParseDuration(value)
 			default:
-				if hooks != nil && hooks.SkipUnknown != nil && hooks.SkipUnknown(section+"."+key, value) {
+				if hooks != nil && hooks.OnUnknown != nil {
+					hooks.OnUnknown(section+"."+key, value)
 					return nil
 				}
-				return errors.Errorf("pebble: unknown option: %s.%s",
-					errors.Safe(section), errors.Safe(key))
+				// Tolerate unknown options, but log them.
+				if o.Logger != nil {
+					o.Logger.Infof("pebble: unknown option: %s.%s", errors.Safe(section), errors.Safe(key))
+				}
+				return nil
 			}
 			return err
 
@@ -2368,24 +2378,38 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 				case "table":
 					l.FilterType = TableFilter
 				default:
-					return errors.Errorf("pebble: unknown filter type: %q", errors.Safe(value))
+					// Tolerate unknown options, but log them.
+					if o.Logger != nil {
+						o.Logger.Infof("pebble: unknown filter type: %q", errors.Safe(value))
+					}
+					return nil
 				}
 			case "index_block_size":
 				l.IndexBlockSize, err = strconv.Atoi(value)
 			case "target_file_size":
 				o.TargetFileSizes[index], err = strconv.ParseInt(value, 10, 64)
 			default:
-				if hooks != nil && hooks.SkipUnknown != nil && hooks.SkipUnknown(section+"."+key, value) {
+				if hooks != nil && hooks.OnUnknown != nil {
+					hooks.OnUnknown(section+"."+key, value)
 					return nil
 				}
-				return errors.Errorf("pebble: unknown option: %s.%s", errors.Safe(section), errors.Safe(key))
+				// Tolerate unknown options, but log them.
+				if o.Logger != nil {
+					o.Logger.Infof("pebble: unknown option: %s.%s", errors.Safe(section), errors.Safe(key))
+				}
+				return nil
 			}
 			return err
 		}
-		if hooks != nil && hooks.SkipUnknown != nil && hooks.SkipUnknown(section+"."+key, value) {
+		if hooks != nil && hooks.OnUnknown != nil {
+			hooks.OnUnknown(section+"."+key, value)
 			return nil
 		}
-		return errors.Errorf("pebble: unknown section %q or key %q", errors.Safe(section), errors.Safe(key))
+		// Tolerate unknown sections and keys, but log them.
+		if o.Logger != nil {
+			o.Logger.Infof("pebble: unknown section %q or key %q", errors.Safe(section), errors.Safe(key))
+		}
+		return nil
 	}
 	err := parseOptions(s, parseOptionsFuncs{
 		visitKeyValue: visitKeyValue,