Add concurrency to backfill-redis #1504
Conversation
Signed-off-by: Cody Soyland <codysoyland@github.com>
codysoyland
force-pushed
the
backfill-redis-concurrency
branch
from
b665f5c to
2a41a4f
Compare
|
/cc @haydentherapper |
Codecov Report
@@ Coverage Diff @@
## main #1504 +/- ##
=======================================
Coverage 67.01% 67.01%
=======================================
Files 82 82
Lines 8325 8325
=======================================
Hits 5579 5579
Misses 2077 2077
Partials 669 669
Flags with carried forward coverage won't be shown. Click here to find out more. |
| if err != nil { | ||
| insertErrs = append(insertErrs, fmt.Errorf("error building index keys for %s: %v", uuid, err)) | ||
| continue | ||
| log.Fatalf("retrieving log uuid by index: %v", err) |
There was a problem hiding this comment.
Can we change this to just print the error?
There was a problem hiding this comment.
We can, but I want to highlight that this is the current behavior (I wish the GitHub diff showed character-level diff so we can see that this whole block was indented and barely modified), and when I ran this on production (over ~50,000 entries), I never hit this. So IMO, I would rather retain this behavior -- bail if the Rekor call fails, since that likely indicates a config/networking problem. Let me know, I'm happy to change it if you disagree.
There was a problem hiding this comment.
Fair point, I'm ok with keeping this as is.
| } | ||
| return nil | ||
| }) | ||
| } |
There was a problem hiding this comment.
Could we also track which indices couldn't be backfilled and log those?
There was a problem hiding this comment.
Not sure if this is what you had in mind, but I added a result summary in my last commit. It does make a simple script a bit more complex, so I'm totally fine removing/changing it.
I decided to separate "parse errors" from "insert errors" when collating results, as I think parse errors are sort of unactionable and insert errors might be recoverable.
The results appear at the end of the command output, for example:
...
Uploaded Redis entry 24296fb24b8ad77abe5d0de220d5e64d0b52178d175a5833a5e327928368a2c9beea87009046c16d, index 21320998, key sha256:ee2ae5b13687d4d443a206359ad7c24a31b753c2233d9d1cb04dbe88535d09db
Completed log index 21320998
Backfill complete
Failed to parse 2 entries: [21320576 21320640]
Failed to insert/remove 1 entries: [21320316]
There was a problem hiding this comment.
This looks great!!
Signed-off-by: Cody Soyland <codysoyland@github.com>
This adds concurrency to the
backfill-rediscommand, including:-concurrencyflag to set worker pool size, defaulting to 1-dry-runflag to test without writing to RedisFixes #1501