-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: add header for skipping the blockcache #152
Conversation
e72840e
to
eb778a2
Compare
|
||
// Process cache skipping header | ||
if noBlockCache := request.Header.Get(NoBlockcacheHeader); noBlockCache != "" { | ||
ds, err := leveldb.NewDatastore("", nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bring in leveldb as a dependency, but IIRC the levelDB memory datastore behaves a bunch better under parallel block writing/fetching load compared to the mapdatastore + sync datastore wrapper. I don't expect this to be used a ton either way, so if people disagree I'm ok switching to the more basic datastore.
(other things landed, I'm going to rebase and review) |
eb778a2
to
316f53c
Compare
16d71e2
to
c0447ab
Compare
Cosmetic cleanup, also moved tracing to dedicated func We also disable debug and tracing when auth token is not set (""), to ensure it is not a DoS vector.
Just a precaution, that we dont open tracing by default when we refactor things going forward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, thank you for fixing the prometheus panic in tests.
Pushed small refactor (moved configuration to Config an related with*
funcs), and also made sure default (when RAINBOW_TRACING_AUTH
is unset) behavior has explicit test.
Merging, will release next.
@@ -71,7 +71,7 @@ func mustTestNodeWithKey(t *testing.T, cfg Config, sk ic.PrivKey) *Node { | |||
func mustTestServer(t *testing.T, cfg Config) (*httptest.Server, *Node) { | |||
nd := mustTestNode(t, cfg) | |||
|
|||
handler, err := setupGatewayHandler(cfg, nd, "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ℹ️ I've moved the token to cfg.TracingAuthToken
// Disable tracing/debug headers if auth token missing or invalid | ||
if authToken == "" || request.Header.Get("Authorization") != authToken { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ℹ️ First check ensures tracing and debug is disabled by default when Authorization
is missing AND RAINBOW_TRACING_AUTH
being empty/unset.
Added a test too, just as a precaution, we don't want to have tracing enabled by default config. We may relax that in the future, but for now better to guard it behind the token, always.
Adds a header (protected by the Authorization header) for skipping the blockcache to better aid debugging.
Based off of #143