Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: update catchpointdump 'database' and 'database check' commands to handle staging tables and KVs #4802

Merged
merged 3 commits into from
Nov 18, 2022

Conversation

cce
Copy link
Contributor

@cce cce commented Nov 16, 2022

Summary

The catchpointdump "database" and "database check" commands did not handle KVs or unlimited assets very well, and this updates them to add a "--staging" flag allow dumping/checking both the catchpoint staging tables and the regular tables.

Test Plan

Tested manually.

@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Merging #4802 (a4e3c6b) into master (23890a8) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4802   +/-   ##
=======================================
  Coverage   54.68%   54.69%           
=======================================
  Files         414      414           
  Lines       53550    53550           
=======================================
+ Hits        29286    29288    +2     
- Misses      21836    21838    +2     
+ Partials     2428     2424    -4     
Impacted Files Coverage Δ
agreement/cryptoVerifier.go 67.60% <0.00%> (-2.12%) ⬇️
agreement/proposalManager.go 96.07% <0.00%> (-1.97%) ⬇️
catchup/service.go 68.14% <0.00%> (-1.24%) ⬇️
network/wsNetwork.go 65.34% <0.00%> (ø)
ledger/testing/randomAccounts.go 57.23% <0.00%> (+0.61%) ⬆️
crypto/merkletrie/node.go 93.48% <0.00%> (+1.86%) ⬆️
crypto/merkletrie/trie.go 68.61% <0.00%> (+2.18%) ⬆️
ledger/roundlru.go 96.22% <0.00%> (+5.66%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines 118 to 121
fmt.Fprintf(outFile, " Root: %s\n", root.String())
if err != nil {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does printing the root even when err != nil make sense?

Also, doesn't String() get called implicitly by %s?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@@ -439,7 +403,7 @@ func printKeyValue(writer *bufio.Writer, key, value []byte) {
fmt.Fprintf(writer, "%s : %v\n", pretty, base64.StdEncoding.EncodeToString(value))
}

func printKeyValueStore(databaseName string, outFile *os.File) error {
func printKeyValueStore(databaseName string, stagingTables bool, fileHeader ledger.CatchpointFileHeader, outFile *os.File) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you just add fileHeader for consistency among the print funcs? It's unused, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm fileHeader is unused. I do not recall why the parameter was added during debugging. Unless @cce recalls, I think it's preferable to remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just removed it, it was left over from before I added the stagingTables arg.

Copy link
Contributor

@jannotti jannotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addresses my minor comments - @algorandskiy had the more substantive questions, so we should merge when he approves.

@cce cce merged commit 60cdd7d into algorand:master Nov 18, 2022
@cce cce deleted the fix-catchpointdump branch November 18, 2022 22:12
algolucky pushed a commit to algolucky/go-algorand that referenced this pull request Dec 15, 2022
… to handle staging tables and KVs (algorand#4802)

* update catchpointdump 'database' and 'database check' commands to handle staging tables and KVs

* address CR comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants