handle an empty namespace in FullScanIterator skip list - levedb

[cendhu]

Type of change

• Improvement (improvement to code)

Description

An empty namespace is used for the config block. However, we would skip only the private data namespaces but not an empty namespace. However, we need to make the function generic.

[manish-sethi]

Actually, these two cases separately are not needed as such. This can simply be

switch {
		case !s.toSkip(ns):
			return compositeKey, dbVal, nil
		default:
                       // default logic
	}

I had added additional explicit check for string equality as an future optimization, where in if we could somehow reuse the same string till the namespace changes. But in the current form, as it's causing to add one more flag, IMO, let's keep the simpler version for now - wdyt?

[cendhu]

switch {
		case !s.toSkip(ns):
			return compositeKey, dbVal, nil
		default:
                       // default logic
	}

Yes, I thought of doing this. There were two choices and both had a + and -.

Choice 1
If we go with the above flow, we would call the following function for every key-value pair in the database but it simplifies the code.

func isPvtdataNs(namespace string) bool {
	return strings.Contains(namespace, nsJoiner+pvtDataPrefix)
}

Choice 2
if we go with the current one, it makes the code bit complex but reduces the number of calls to isPvtdataNs

switch {
		case s.isCurrentNamespaceSet && ns == s.currentNamespace:
			return compositeKey, dbVal, nil
		case !s.toSkip(ns):
			s.isCurrentNamespaceSet = true
			s.currentNamespace = ns
			return compositeKey, dbVal, nil
}

As you prefer choice 1, I will go with that as I don't have a strong preference here.

[cendhu]

Done. Later, if needed (after the performance benchmarking), we can add a method toSkipNamespaces() on the scanner and include a map to avoid calling isPvtdataNs() for every entry in the database.

[manish-sethi]

it makes the code bit complex but reduces the number of calls to isPvtdataNs
Just for a note, this does not make any difference until the string comparison in the other case is optimized.

[cendhu]

Thanks, Manish, for merging this. I quickly ran a benchmark.

BenchmarkIsPvtNs-8      140749465                8.43 ns/op

It took 1.18sec for more than 100 million calls. Hence, it isn't an issue.