cli: Add 'pebble' debug command to run Pebble tool commands

[itsbilal]

Command-ception: Add a debug pebble command similar to debug rocksdb that nests pebble commands like sstable scan, manifest
dump, etc right in the cockroach binary.

Also move the pebble.Compare definition to engine, which is a more
fitting place for it than bulk.

Fixes #40509

Release note: None

Release justification: Adds debug tools for internal use, no impact
on general operation.

[cockroach-teamcity]

This change is 

[petermattis]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajkr, @itsbilal, and @petermattis)

---

pkg/cli/debug.go, line 765 at r1 (raw file):

var debugPebbleCmd = &cobra.Command{
	Use:   "pebble [command]",
	Short: "run a Pebble tool command",

Nit: s/Pebble tool/Pebble introspection tool/g

---

pkg/cli/debug.go, line 769 at r1 (raw file):

Allows the use of pebble tools, such as to introspect manifests, SSTables, etc.
'cockroach debug pebble' accepts the same arguments and flags as the 'pebble'
binary.

I'd leave out the bit about "the same arguments and flags as the 'pebble' binary". The user probably doesn't know about that binary.

---

pkg/cli/debug.go, line 1337 at r1 (raw file):

	pebbleTool := tool.New()
	merger := *pebble.DefaultMerger
	merger.Name = "cockroach_merge_operator"

This isn't correct and deserves a TODO(bilal). I'm guessing some point soon you'll have the cockroach merge operator ported from C++ back to Go. Also add a note that this means that any timeseries data will be displayed incorrectly (concatenated instead of "merged").

---

pkg/storage/engine/mvcc.go, line 1868 at r1 (raw file):

// comparator settings for use with Pebble.
//
// TODO(itsbilal): Move this to a new file pebble.go.

Might as well create pebble.go now, or are you worried about a conflict with something else?

---

pkg/storage/engine/mvcc.go, line 1878 at r1 (raw file):

		return pebble.DefaultComparer.AbbreviatedKey(key)
	},

Let's fill in Format. You should be able to do something like:

func(k []byte) fmt.Formatter {
  return roachpb.Key(k)
}

I think that should just work, but if it doesn't I can help figure out what needs to be done. Once this is done, can you check that pebble sstable scan --key=%s for a cockroach generated sstable pretty-prints the keys?

[itsbilal]

Now that cockroachdb/pebble#267 has landed, this can go through. PTAL!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajkr and @petermattis)

---

pkg/cli/debug.go, line 769 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

I'd leave out the bit about "the same arguments and flags as the 'pebble' binary". The user probably doesn't know about that binary.

Done.

---

pkg/cli/debug.go, line 1337 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

This isn't correct and deserves a TODO(bilal). I'm guessing some point soon you'll have the cockroach merge operator ported from C++ back to Go. Also add a note that this means that any timeseries data will be displayed incorrectly (concatenated instead of "merged").

Yep, I am doing the port as part of my work for #39674. For now I'll add a TODO describing that. Is that enough or are we going to need to update the command help strings to also mention the discrepancy?

---

pkg/storage/engine/mvcc.go, line 1868 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Might as well create pebble.go now, or are you worried about a conflict with something else?

I could - really the only conflict is with my own work in #39674 when that lands soon.

---

pkg/storage/engine/mvcc.go, line 1878 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Let's fill in Format. You should be able to do something like:

func(k []byte) fmt.Formatter {
  return roachpb.Key(k)
}

I think that should just work, but if it doesn't I can help figure out what needs to be done. Once this is done, can you check that pebble sstable scan --key=%s for a cockroach generated sstable pretty-prints the keys?

Done. Confirmed that the pretty-printing works, with --format=pretty.

[petermattis]

LGTM modulo the comment on MVCCKey.Format.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajkr and @itsbilal)

---

pkg/storage/engine/mvcc.go, line 1868 at r1 (raw file):

Previously, itsbilal (Bilal Akhtar) wrote…

I could - really the only conflict is with my own work in #39674 when that lands soon.

Ack. Ok.

---

pkg/storage/engine/mvcc.go, line 130 at r2 (raw file):

// Format implements the fmt.Formatter interface.
func (k MVCCKey) Format(f fmt.State, c rune) {
	fmt.Fprintf(f, "%s/%s", keys.PrettyPrint(nil, k.Key), k.Timestamp)

Is the keys.PrettyPrint call necessary? I think you can pass k.Key to fmt.Fprintf and it will automatically call k.Key.String() which will in turn call PrettyPrint.

[itsbilal]

Made the fix (plus other linter-found issues). Thanks for the review!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajkr, @itsbilal, and @petermattis)

---

pkg/storage/engine/mvcc.go, line 130 at r2 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Is the keys.PrettyPrint call necessary? I think you can pass k.Key to fmt.Fprintf and it will automatically call k.Key.String() which will in turn call PrettyPrint.

Yes, it's unnecessary. Removed it.

[itsbilal]

bors r+

[craig]

Build failed (retrying...)

• GitHub CI (Cockroach)

[craig]

Build failed

• GitHub CI (Cockroach)

[itsbilal]

Release justification copied to PR description.

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)

[petermattis]

@jseldess Do we document the cockroach debug rocksdb command? I couldn't find anything, but wanted to double-check. The new debug pebble commands are preferable from an ease of use standpoint.