Add Request Timing Controls for Auth Webhook Client #1142
Conversation
- remove cache
- body is closed in retryablehttp.drainBody(resp.Body)
- AuthWebhookMinWaitInterval - AuthWebhookRequestTimeout
- for distinguish status code 0 and shouldRetry status codes
- DefaultAuthWebhookMinWaitInterval - DefaultAuthWebhookRequestTimeout
WalkthroughThis pull request extends the authentication webhook configuration across multiple modules. It introduces new CLI flags, configuration fields, and default values for setting the minimum wait interval and request timeout for webhook requests. The webhook client logic has been enhanced with signature generation and refined error handling. Additionally, related tests and configuration validations have been updated to cover the new parameters. Overall, the changes integrate the extended webhook settings into the server initialization and RPC authentication flow. Changes
Sequence Diagram(s)sequenceDiagram
participant ARH as Auth Request Handler
participant WC as Webhook Client
participant WS as Webhook Server
participant Cache as Response Cache
ARH->>WC: Marshal AuthWebhookRequest to JSON
WC->>WS: Send HTTP Request with HMAC signature
WS-->>WC: Return HTTP response
WC->>ARH: Return status and AuthWebhookResponse
ARH->>ARH: Invoke handleWebhookResponse to process result
ARH->>Cache: Cache response (if applicable)
Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
server/rpc/auth/webhook.go (1)
75-75: Address the TODO: Consider caching unauthorized responsesThere's a TODO comment indicating the need to consider caching responses with
http.StatusUnauthorized:// TODO(hackerwins): We should consider caching the response of Unauthorized as well.Caching unauthorized responses could improve performance by reducing repetitive authentication attempts for invalid tokens. Would you like assistance in implementing this feature?
pkg/webhook/client.go (2)
142-151: Clarify handling of empty HMAC key increateSignatureIn the
createSignaturefunction, when thehmacKeyis empty, the function returns an empty string without an error:if hmacKey == "" { return "", nil }While this might be intentional, it may lead to confusion about whether HMAC signing is required. Consider documenting this behavior or explicitly handling cases where HMAC authentication is optional.
195-205: Ensure cross-platform compatibility inshouldRetryerror handlingThe
shouldRetryfunction checks for connection reset errors usingsyscall.Errno:var errno syscall.Errno if errors.As(err, &errno) { return errors.Is(errno, syscall.ECONNRESET) }On non-Unix systems like Windows,
syscall.ECONNRESETmay not behave as expected. Consider using thenetpackage'snet.Errorinterface to check for temporary network errors in a platform-agnostic way.Apply this diff to update the error handling:
import ( "net" // ... ) func shouldRetry(statusCode int, err error) bool { + if err != nil { + var netErr net.Error + if errors.As(err, &netErr) && netErr.Temporary() { + return true + } + } return statusCode == http.StatusInternalServerError || statusCode == http.StatusServiceUnavailable || statusCode == http.StatusGatewayTimeout || statusCode == http.StatusTooManyRequests }pkg/webhook/client_test.go (1)
208-248: Reduce test flakiness caused by timing dependenciesThe tests
TestRequestTimeoutandTestRetryRequestdepend on precise timing, which can lead to flakiness in environments with variable performance:delayTime := 10 * time.Millisecond // ... RequestTimeout: 5 * time.Millisecond,Consider increasing timeouts or refactoring tests to be less sensitive to timing, such as using synchronization mechanisms or mocking time-dependent functions.
server/backend/backend.go (1)
93-100: Consider adding error handling for webhook client initialization.While the webhook client initialization looks correct, consider adding error handling in case the configuration parsing fails or the client initialization encounters issues.
- auth := webhook.NewClient[types.AuthWebhookRequest, types.AuthWebhookResponse]( + auth, err := webhook.NewClient[types.AuthWebhookRequest, types.AuthWebhookResponse]( webhook.Options{ MaxRetries: conf.AuthWebhookMaxRetries, MinWaitInterval: conf.ParseAuthWebhookMinWaitInterval(), MaxWaitInterval: conf.ParseAuthWebhookMaxWaitInterval(), RequestTimeout: conf.ParseAuthWebhookRequestTimeout(), }, ) + if err != nil { + return nil, fmt.Errorf("failed to create webhook client: %w", err) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
cmd/yorkie/server.go(3 hunks)pkg/webhook/client.go(5 hunks)pkg/webhook/client_test.go(3 hunks)server/backend/backend.go(4 hunks)server/backend/config.go(3 hunks)server/backend/config_test.go(2 hunks)server/config.go(2 hunks)server/config.sample.yml(1 hunks)server/config_test.go(1 hunks)server/packs/packs_test.go(1 hunks)server/rpc/auth/webhook.go(2 hunks)server/rpc/server_test.go(1 hunks)test/complex/main_test.go(1 hunks)test/helper/helper.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: bench
- GitHub Check: build
🔇 Additional comments (15)
server/rpc/auth/webhook.go (1)
68-68: Verify HMAC key usage in webhook client send methodIn the call to
be.WebhookClient.Send, you're passing an empty string for thehmacKeyparameter:res, status, err := be.WebhookClient.Send( ctx, prj.AuthWebhookURL, "", body, )If the authentication webhook requires HMAC signing for requests, providing an empty string may lead to unauthorized access errors. Please verify whether the HMAC key should be supplied here.
pkg/webhook/client.go (1)
183-190: Verify exponential backoff calculation inwaitIntervalThe
waitIntervalfunction calculates the delay between retries exponentially:interval := time.Duration(math.Pow(2, float64(retries))) * minWaitIntervalEnsure that
minWaitIntervalandmaxWaitIntervalare configured appropriately to prevent excessively long or short wait times, especially in scenarios with high retry counts.✅ Verification successful
Exponential Backoff Calculation Verified
The calculation in
waitIntervalcorrectly computes an interval by exponentially scaling theminWaitInterval(using 2^retries) and then clamps the value to not exceedmaxWaitInterval. Just ensure that the configuration values forminWaitIntervalandmaxWaitIntervalare chosen to suit the operational context.server/backend/config_test.go (2)
32-34: LGTM! Validation test configuration is properly structured.The new configuration fields are correctly added to the valid configuration object with appropriate default values.
47-53: LGTM! Validation test cases are comprehensive.The test cases properly verify that invalid duration formats are rejected for both new configuration fields.
server/config_test.go (1)
73-79: LGTM! Default value tests are properly implemented.The tests correctly verify that the new configuration fields match their default values and include proper error handling.
test/complex/main_test.go (1)
82-82: LGTM! Configuration parameter properly added.The AuthWebhookCacheTTL is correctly added to the backend configuration with proper string conversion.
server/backend/backend.go (2)
52-53: LGTM! WebhookClient field is properly documented.The new WebhookClient field is well-documented and properly typed.
157-163: LGTM! Backend struct initialization is properly ordered.The initialization of the Backend struct maintains a logical order with the new webhook-related fields grouped together.
server/backend/config.go (1)
70-71: LGTM!The field declaration and comment for
AuthWebhookRequestTimeoutare clear and accurate.server/config.go (1)
65-66: LGTM!The default values for
DefaultAuthWebhookMinWaitInterval(100ms) andDefaultAuthWebhookRequestTimeout(10s) are reasonable and align with the PR objectives.server/rpc/server_test.go (1)
79-80: LGTM!The new configuration parameters are correctly added and follow the same pattern as other parameters.
cmd/yorkie/server.go (1)
51-52: LGTM!The new variables and flags are correctly implemented:
- Variables are properly declared as
time.Duration.- Flags use appropriate default values from the server package.
- Help messages are clear and descriptive.
- Values are correctly assigned to the configuration.
Also applies to: 69-70, 314-325
server/packs/packs_test.go (1)
103-106: LGTM! Test configuration updated with new auth webhook settings.The test configuration has been correctly updated to include the new auth webhook configuration fields, using values from the helper package.
test/helper/helper.go (1)
78-79: Verify test values for auth webhook settings.The test values are significantly lower than the production values in
config.sample.yml:
- Test:
AuthWebhookMinWaitInterval = 3msvs Production:200ms- Test:
AuthWebhookRequestTimeout = 100msvs Production:30sPlease confirm if these lower values are intentional for testing purposes.
server/config.sample.yml (1)
74-79: LGTM! Well-documented configuration options with reasonable defaults.The new auth webhook configuration options are:
- Well-documented with clear descriptions
- Have reasonable default values for production use
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server/rpc/auth/webhook.go (1)
60-63: Consider adding cache expiration handling.While the cache lookup is implemented correctly, consider handling cache entry expiration explicitly to prevent using stale entries.
if entry, ok := be.WebhookCache.Get(cacheKey); ok { + if time.Since(entry.Timestamp) < be.Config.ParseAuthWebhookCacheTTL() { return handleWebhookResponse(entry.First, entry.Second) + } }server/backend/config.go (1)
110-124: Consider adding validation for minimum values.While the duration parsing is correct, consider adding validation for minimum acceptable values to prevent performance issues.
if _, err := time.ParseDuration(c.AuthWebhookMinWaitInterval); err != nil { return fmt.Errorf( `invalid argument "%s" for "--auth-webhook-min-wait-interval" flag: %w`, c.AuthWebhookMinWaitInterval, err, ) } +minWait, _ := time.ParseDuration(c.AuthWebhookMinWaitInterval) +if minWait < 50*time.Millisecond { + return fmt.Errorf("min wait interval must be at least 50ms") +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
pkg/webhook/client_test.go(3 hunks)server/backend/config.go(3 hunks)server/config.sample.yml(1 hunks)server/rpc/auth/webhook.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/config.sample.yml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: bench
- GitHub Check: build
🔇 Additional comments (8)
server/rpc/auth/webhook.go (4)
49-53: LGTM!The request struct initialization is clean and follows the standard pattern.
55-58: LGTM!The JSON marshaling error handling is appropriate and follows best practices.
94-96: LGTM!The nil check for the response is correctly implemented, addressing the previous review comment about potential nil pointer dereference.
75-82: Consider caching unauthorized responses.The TODO comment suggests caching unauthorized responses. This could be beneficial for performance but needs careful consideration of security implications.
Let's verify the impact of caching unauthorized responses:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for potential security implications of caching unauthorized responses # Test: Search for patterns where unauthorized responses are handled differently rg -A 5 "StatusUnauthorized|ErrUnauthenticated"Length of output: 6203
Security Stance on Unauthorized Response Caching
The investigation confirms that the codebase intentionally excludes unauthorized responses from being cached (see
if status != http.StatusUnauthorizedinserver/rpc/auth/webhook.go). This pattern is consistent with tests and other components (e.g., integration tests checking forErrUnauthenticated), suggesting that caching such responses was deliberately avoided to mitigate potential security and correctness risks.
- The authorization flow and tests indicate that unauthorized responses are meant to be handled freshly, avoiding stale error states.
- Caching unauthorized responses could lead to unintended side effects (e.g., caching transient errors), which may introduce security concerns.
Overall, the current approach appears to be a conscious design decision rather than an oversight.
server/backend/config.go (1)
67-71: LGTM!The configuration fields are well-documented and follow the established pattern.
pkg/webhook/client_test.go (3)
33-43: LGTM!The signature verification is secure and follows cryptographic best practices.
87-99: Optimize test execution time.The delay server implementation uses real time delays which can slow down the test suite.
Consider using a mock time source:
+type mockClock struct { + now time.Time +} + +func (m *mockClock) Sleep(d time.Duration) { + m.now = m.now.Add(d) +} + func newDelayServer(t *testing.T, delayTime time.Duration, responseData testResponse) *httptest.Server { + clock := &mockClock{now: time.Now()} return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx, cancel := context.WithTimeout(r.Context(), delayTime) - defer cancel() - - select { - case <-ctx.Done(): - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - assert.NoError(t, json.NewEncoder(w).Encode(responseData)) - } + clock.Sleep(delayTime) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + assert.NoError(t, json.NewEncoder(w).Encode(responseData)) })) }
164-209: LGTM!The retry and timeout tests are comprehensive and cover important edge cases.
Also applies to: 211-253
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Go Benchmark
| Benchmark suite | Current: 638932f | Previous: 871dcc7 | Ratio |
|---|---|---|---|
BenchmarkDocument/constructor_test |
1433 ns/op 1385 B/op 24 allocs/op |
1420 ns/op 1385 B/op 24 allocs/op |
1.01 |
BenchmarkDocument/constructor_test - ns/op |
1433 ns/op |
1420 ns/op |
1.01 |
BenchmarkDocument/constructor_test - B/op |
1385 B/op |
1385 B/op |
1 |
BenchmarkDocument/constructor_test - allocs/op |
24 allocs/op |
24 allocs/op |
1 |
BenchmarkDocument/status_test |
1002 ns/op 1353 B/op 22 allocs/op |
1008 ns/op 1353 B/op 22 allocs/op |
0.99 |
BenchmarkDocument/status_test - ns/op |
1002 ns/op |
1008 ns/op |
0.99 |
BenchmarkDocument/status_test - B/op |
1353 B/op |
1353 B/op |
1 |
BenchmarkDocument/status_test - allocs/op |
22 allocs/op |
22 allocs/op |
1 |
BenchmarkDocument/equals_test |
8822 ns/op 7562 B/op 129 allocs/op |
7675 ns/op 7562 B/op 129 allocs/op |
1.15 |
BenchmarkDocument/equals_test - ns/op |
8822 ns/op |
7675 ns/op |
1.15 |
BenchmarkDocument/equals_test - B/op |
7562 B/op |
7562 B/op |
1 |
BenchmarkDocument/equals_test - allocs/op |
129 allocs/op |
129 allocs/op |
1 |
BenchmarkDocument/nested_update_test |
17070 ns/op 12308 B/op 258 allocs/op |
16848 ns/op 12307 B/op 258 allocs/op |
1.01 |
BenchmarkDocument/nested_update_test - ns/op |
17070 ns/op |
16848 ns/op |
1.01 |
BenchmarkDocument/nested_update_test - B/op |
12308 B/op |
12307 B/op |
1.00 |
BenchmarkDocument/nested_update_test - allocs/op |
258 allocs/op |
258 allocs/op |
1 |
BenchmarkDocument/delete_test |
23807 ns/op 15788 B/op 339 allocs/op |
23012 ns/op 15788 B/op 339 allocs/op |
1.03 |
BenchmarkDocument/delete_test - ns/op |
23807 ns/op |
23012 ns/op |
1.03 |
BenchmarkDocument/delete_test - B/op |
15788 B/op |
15788 B/op |
1 |
BenchmarkDocument/delete_test - allocs/op |
339 allocs/op |
339 allocs/op |
1 |
BenchmarkDocument/object_test |
8924 ns/op 7033 B/op 118 allocs/op |
8789 ns/op 7034 B/op 118 allocs/op |
1.02 |
BenchmarkDocument/object_test - ns/op |
8924 ns/op |
8789 ns/op |
1.02 |
BenchmarkDocument/object_test - B/op |
7033 B/op |
7034 B/op |
1.00 |
BenchmarkDocument/object_test - allocs/op |
118 allocs/op |
118 allocs/op |
1 |
BenchmarkDocument/array_test |
30007 ns/op 12139 B/op 273 allocs/op |
28459 ns/op 12139 B/op 273 allocs/op |
1.05 |
BenchmarkDocument/array_test - ns/op |
30007 ns/op |
28459 ns/op |
1.05 |
BenchmarkDocument/array_test - B/op |
12139 B/op |
12139 B/op |
1 |
BenchmarkDocument/array_test - allocs/op |
273 allocs/op |
273 allocs/op |
1 |
BenchmarkDocument/text_test |
32593 ns/op 15188 B/op 484 allocs/op |
38480 ns/op 15189 B/op 484 allocs/op |
0.85 |
BenchmarkDocument/text_test - ns/op |
32593 ns/op |
38480 ns/op |
0.85 |
BenchmarkDocument/text_test - B/op |
15188 B/op |
15189 B/op |
1.00 |
BenchmarkDocument/text_test - allocs/op |
484 allocs/op |
484 allocs/op |
1 |
BenchmarkDocument/text_composition_test |
31966 ns/op 18704 B/op 501 allocs/op |
31410 ns/op 18702 B/op 501 allocs/op |
1.02 |
BenchmarkDocument/text_composition_test - ns/op |
31966 ns/op |
31410 ns/op |
1.02 |
BenchmarkDocument/text_composition_test - B/op |
18704 B/op |
18702 B/op |
1.00 |
BenchmarkDocument/text_composition_test - allocs/op |
501 allocs/op |
501 allocs/op |
1 |
BenchmarkDocument/rich_text_test |
89149 ns/op 39352 B/op 1146 allocs/op |
86111 ns/op 39359 B/op 1146 allocs/op |
1.04 |
BenchmarkDocument/rich_text_test - ns/op |
89149 ns/op |
86111 ns/op |
1.04 |
BenchmarkDocument/rich_text_test - B/op |
39352 B/op |
39359 B/op |
1.00 |
BenchmarkDocument/rich_text_test - allocs/op |
1146 allocs/op |
1146 allocs/op |
1 |
BenchmarkDocument/counter_test |
19121 ns/op 11811 B/op 253 allocs/op |
18099 ns/op 11811 B/op 253 allocs/op |
1.06 |
BenchmarkDocument/counter_test - ns/op |
19121 ns/op |
18099 ns/op |
1.06 |
BenchmarkDocument/counter_test - B/op |
11811 B/op |
11811 B/op |
1 |
BenchmarkDocument/counter_test - allocs/op |
253 allocs/op |
253 allocs/op |
1 |
BenchmarkDocument/text_edit_gc_100 |
1427785 ns/op 864926 B/op 17282 allocs/op |
1388642 ns/op 864885 B/op 17282 allocs/op |
1.03 |
BenchmarkDocument/text_edit_gc_100 - ns/op |
1427785 ns/op |
1388642 ns/op |
1.03 |
BenchmarkDocument/text_edit_gc_100 - B/op |
864926 B/op |
864885 B/op |
1.00 |
BenchmarkDocument/text_edit_gc_100 - allocs/op |
17282 allocs/op |
17282 allocs/op |
1 |
BenchmarkDocument/text_edit_gc_1000 |
54149998 ns/op 46838920 B/op 185591 allocs/op |
51278183 ns/op 46838150 B/op 185588 allocs/op |
1.06 |
BenchmarkDocument/text_edit_gc_1000 - ns/op |
54149998 ns/op |
51278183 ns/op |
1.06 |
BenchmarkDocument/text_edit_gc_1000 - B/op |
46838920 B/op |
46838150 B/op |
1.00 |
BenchmarkDocument/text_edit_gc_1000 - allocs/op |
185591 allocs/op |
185588 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_100 |
2152613 ns/op 1580972 B/op 15950 allocs/op |
2101223 ns/op 1581113 B/op 15951 allocs/op |
1.02 |
BenchmarkDocument/text_split_gc_100 - ns/op |
2152613 ns/op |
2101223 ns/op |
1.02 |
BenchmarkDocument/text_split_gc_100 - B/op |
1580972 B/op |
1581113 B/op |
1.00 |
BenchmarkDocument/text_split_gc_100 - allocs/op |
15950 allocs/op |
15951 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_1000 |
131248535 ns/op 137790572 B/op 184995 allocs/op |
128023140 ns/op 137788096 B/op 184992 allocs/op |
1.03 |
BenchmarkDocument/text_split_gc_1000 - ns/op |
131248535 ns/op |
128023140 ns/op |
1.03 |
BenchmarkDocument/text_split_gc_1000 - B/op |
137790572 B/op |
137788096 B/op |
1.00 |
BenchmarkDocument/text_split_gc_1000 - allocs/op |
184995 allocs/op |
184992 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_10000 |
19968060 ns/op 10576474 B/op 56135 allocs/op |
17172481 ns/op 10577269 B/op 56138 allocs/op |
1.16 |
BenchmarkDocument/text_delete_all_10000 - ns/op |
19968060 ns/op |
17172481 ns/op |
1.16 |
BenchmarkDocument/text_delete_all_10000 - B/op |
10576474 B/op |
10577269 B/op |
1.00 |
BenchmarkDocument/text_delete_all_10000 - allocs/op |
56135 allocs/op |
56138 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 |
310049508 ns/op 105574212 B/op 566108 allocs/op |
254778026 ns/op 105506856 B/op 566032 allocs/op |
1.22 |
BenchmarkDocument/text_delete_all_100000 - ns/op |
310049508 ns/op |
254778026 ns/op |
1.22 |
BenchmarkDocument/text_delete_all_100000 - B/op |
105574212 B/op |
105506856 B/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 - allocs/op |
566108 allocs/op |
566032 allocs/op |
1.00 |
BenchmarkDocument/text_100 |
232992 ns/op 120907 B/op 5181 allocs/op |
227567 ns/op 120942 B/op 5181 allocs/op |
1.02 |
BenchmarkDocument/text_100 - ns/op |
232992 ns/op |
227567 ns/op |
1.02 |
BenchmarkDocument/text_100 - B/op |
120907 B/op |
120942 B/op |
1.00 |
BenchmarkDocument/text_100 - allocs/op |
5181 allocs/op |
5181 allocs/op |
1 |
BenchmarkDocument/text_1000 |
2501989 ns/op 1156064 B/op 51084 allocs/op |
2441326 ns/op 1156078 B/op 51084 allocs/op |
1.02 |
BenchmarkDocument/text_1000 - ns/op |
2501989 ns/op |
2441326 ns/op |
1.02 |
BenchmarkDocument/text_1000 - B/op |
1156064 B/op |
1156078 B/op |
1.00 |
BenchmarkDocument/text_1000 - allocs/op |
51084 allocs/op |
51084 allocs/op |
1 |
BenchmarkDocument/array_1000 |
1264630 ns/op 1088277 B/op 11879 allocs/op |
1229986 ns/op 1088143 B/op 11879 allocs/op |
1.03 |
BenchmarkDocument/array_1000 - ns/op |
1264630 ns/op |
1229986 ns/op |
1.03 |
BenchmarkDocument/array_1000 - B/op |
1088277 B/op |
1088143 B/op |
1.00 |
BenchmarkDocument/array_1000 - allocs/op |
11879 allocs/op |
11879 allocs/op |
1 |
BenchmarkDocument/array_10000 |
13837305 ns/op 9888095 B/op 120732 allocs/op |
13179967 ns/op 9889091 B/op 120735 allocs/op |
1.05 |
BenchmarkDocument/array_10000 - ns/op |
13837305 ns/op |
13179967 ns/op |
1.05 |
BenchmarkDocument/array_10000 - B/op |
9888095 B/op |
9889091 B/op |
1.00 |
BenchmarkDocument/array_10000 - allocs/op |
120732 allocs/op |
120735 allocs/op |
1.00 |
BenchmarkDocument/array_gc_100 |
132237 ns/op 99888 B/op 1266 allocs/op |
131508 ns/op 99890 B/op 1266 allocs/op |
1.01 |
BenchmarkDocument/array_gc_100 - ns/op |
132237 ns/op |
131508 ns/op |
1.01 |
BenchmarkDocument/array_gc_100 - B/op |
99888 B/op |
99890 B/op |
1.00 |
BenchmarkDocument/array_gc_100 - allocs/op |
1266 allocs/op |
1266 allocs/op |
1 |
BenchmarkDocument/array_gc_1000 |
1448936 ns/op 1140921 B/op 12926 allocs/op |
1415346 ns/op 1140837 B/op 12926 allocs/op |
1.02 |
BenchmarkDocument/array_gc_1000 - ns/op |
1448936 ns/op |
1415346 ns/op |
1.02 |
BenchmarkDocument/array_gc_1000 - B/op |
1140921 B/op |
1140837 B/op |
1.00 |
BenchmarkDocument/array_gc_1000 - allocs/op |
12926 allocs/op |
12926 allocs/op |
1 |
BenchmarkDocument/counter_1000 |
203810 ns/op 178137 B/op 5771 allocs/op |
198298 ns/op 178135 B/op 5771 allocs/op |
1.03 |
BenchmarkDocument/counter_1000 - ns/op |
203810 ns/op |
198298 ns/op |
1.03 |
BenchmarkDocument/counter_1000 - B/op |
178137 B/op |
178135 B/op |
1.00 |
BenchmarkDocument/counter_1000 - allocs/op |
5771 allocs/op |
5771 allocs/op |
1 |
BenchmarkDocument/counter_10000 |
2192889 ns/op 2068941 B/op 59778 allocs/op |
2164012 ns/op 2068953 B/op 59778 allocs/op |
1.01 |
BenchmarkDocument/counter_10000 - ns/op |
2192889 ns/op |
2164012 ns/op |
1.01 |
BenchmarkDocument/counter_10000 - B/op |
2068941 B/op |
2068953 B/op |
1.00 |
BenchmarkDocument/counter_10000 - allocs/op |
59778 allocs/op |
59778 allocs/op |
1 |
BenchmarkDocument/object_1000 |
1440762 ns/op 1437237 B/op 9925 allocs/op |
1418168 ns/op 1437251 B/op 9925 allocs/op |
1.02 |
BenchmarkDocument/object_1000 - ns/op |
1440762 ns/op |
1418168 ns/op |
1.02 |
BenchmarkDocument/object_1000 - B/op |
1437237 B/op |
1437251 B/op |
1.00 |
BenchmarkDocument/object_1000 - allocs/op |
9925 allocs/op |
9925 allocs/op |
1 |
BenchmarkDocument/object_10000 |
15525441 ns/op 12350696 B/op 101229 allocs/op |
14752449 ns/op 12351650 B/op 101230 allocs/op |
1.05 |
BenchmarkDocument/object_10000 - ns/op |
15525441 ns/op |
14752449 ns/op |
1.05 |
BenchmarkDocument/object_10000 - B/op |
12350696 B/op |
12351650 B/op |
1.00 |
BenchmarkDocument/object_10000 - allocs/op |
101229 allocs/op |
101230 allocs/op |
1.00 |
BenchmarkDocument/tree_100 |
1082905 ns/op 951036 B/op 6102 allocs/op |
1003575 ns/op 951028 B/op 6102 allocs/op |
1.08 |
BenchmarkDocument/tree_100 - ns/op |
1082905 ns/op |
1003575 ns/op |
1.08 |
BenchmarkDocument/tree_100 - B/op |
951036 B/op |
951028 B/op |
1.00 |
BenchmarkDocument/tree_100 - allocs/op |
6102 allocs/op |
6102 allocs/op |
1 |
BenchmarkDocument/tree_1000 |
84839213 ns/op 86581368 B/op 60112 allocs/op |
75285903 ns/op 86582305 B/op 60112 allocs/op |
1.13 |
BenchmarkDocument/tree_1000 - ns/op |
84839213 ns/op |
75285903 ns/op |
1.13 |
BenchmarkDocument/tree_1000 - B/op |
86581368 B/op |
86582305 B/op |
1.00 |
BenchmarkDocument/tree_1000 - allocs/op |
60112 allocs/op |
60112 allocs/op |
1 |
BenchmarkDocument/tree_10000 |
9922522231 ns/op 8581200992 B/op 600194 allocs/op |
9433915602 ns/op 8581180576 B/op 600183 allocs/op |
1.05 |
BenchmarkDocument/tree_10000 - ns/op |
9922522231 ns/op |
9433915602 ns/op |
1.05 |
BenchmarkDocument/tree_10000 - B/op |
8581200992 B/op |
8581180576 B/op |
1.00 |
BenchmarkDocument/tree_10000 - allocs/op |
600194 allocs/op |
600183 allocs/op |
1.00 |
BenchmarkDocument/tree_delete_all_1000 |
80018414 ns/op 87566541 B/op 75288 allocs/op |
77584693 ns/op 87567259 B/op 75289 allocs/op |
1.03 |
BenchmarkDocument/tree_delete_all_1000 - ns/op |
80018414 ns/op |
77584693 ns/op |
1.03 |
BenchmarkDocument/tree_delete_all_1000 - B/op |
87566541 B/op |
87567259 B/op |
1.00 |
BenchmarkDocument/tree_delete_all_1000 - allocs/op |
75288 allocs/op |
75289 allocs/op |
1.00 |
BenchmarkDocument/tree_edit_gc_100 |
3903909 ns/op 4147826 B/op 15147 allocs/op |
3808966 ns/op 4147894 B/op 15147 allocs/op |
1.02 |
BenchmarkDocument/tree_edit_gc_100 - ns/op |
3903909 ns/op |
3808966 ns/op |
1.02 |
BenchmarkDocument/tree_edit_gc_100 - B/op |
4147826 B/op |
4147894 B/op |
1.00 |
BenchmarkDocument/tree_edit_gc_100 - allocs/op |
15147 allocs/op |
15147 allocs/op |
1 |
BenchmarkDocument/tree_edit_gc_1000 |
325516875 ns/op 384044340 B/op 154953 allocs/op |
310812696 ns/op 384044174 B/op 154947 allocs/op |
1.05 |
BenchmarkDocument/tree_edit_gc_1000 - ns/op |
325516875 ns/op |
310812696 ns/op |
1.05 |
BenchmarkDocument/tree_edit_gc_1000 - B/op |
384044340 B/op |
384044174 B/op |
1.00 |
BenchmarkDocument/tree_edit_gc_1000 - allocs/op |
154953 allocs/op |
154947 allocs/op |
1.00 |
BenchmarkDocument/tree_split_gc_100 |
2671356 ns/op 2407300 B/op 11131 allocs/op |
2564814 ns/op 2407295 B/op 11131 allocs/op |
1.04 |
BenchmarkDocument/tree_split_gc_100 - ns/op |
2671356 ns/op |
2564814 ns/op |
1.04 |
BenchmarkDocument/tree_split_gc_100 - B/op |
2407300 B/op |
2407295 B/op |
1.00 |
BenchmarkDocument/tree_split_gc_100 - allocs/op |
11131 allocs/op |
11131 allocs/op |
1 |
BenchmarkDocument/tree_split_gc_1000 |
196337402 ns/op 222499088 B/op 122075 allocs/op |
187969114 ns/op 222500200 B/op 122078 allocs/op |
1.04 |
BenchmarkDocument/tree_split_gc_1000 - ns/op |
196337402 ns/op |
187969114 ns/op |
1.04 |
BenchmarkDocument/tree_split_gc_1000 - B/op |
222499088 B/op |
222500200 B/op |
1.00 |
BenchmarkDocument/tree_split_gc_1000 - allocs/op |
122075 allocs/op |
122078 allocs/op |
1.00 |
BenchmarkRPC/client_to_server |
431490019 ns/op 16451712 B/op 223953 allocs/op |
417816374 ns/op 16158578 B/op 223875 allocs/op |
1.03 |
BenchmarkRPC/client_to_server - ns/op |
431490019 ns/op |
417816374 ns/op |
1.03 |
BenchmarkRPC/client_to_server - B/op |
16451712 B/op |
16158578 B/op |
1.02 |
BenchmarkRPC/client_to_server - allocs/op |
223953 allocs/op |
223875 allocs/op |
1.00 |
BenchmarkRPC/client_to_client_via_server |
816541200 ns/op 35908752 B/op 476657 allocs/op |
783154610 ns/op 36375760 B/op 476970 allocs/op |
1.04 |
BenchmarkRPC/client_to_client_via_server - ns/op |
816541200 ns/op |
783154610 ns/op |
1.04 |
BenchmarkRPC/client_to_client_via_server - B/op |
35908752 B/op |
36375760 B/op |
0.99 |
BenchmarkRPC/client_to_client_via_server - allocs/op |
476657 allocs/op |
476970 allocs/op |
1.00 |
BenchmarkRPC/attach_large_document |
1235268250 ns/op 1922037360 B/op 12428 allocs/op |
1254422570 ns/op 1897382304 B/op 12568 allocs/op |
0.98 |
BenchmarkRPC/attach_large_document - ns/op |
1235268250 ns/op |
1254422570 ns/op |
0.98 |
BenchmarkRPC/attach_large_document - B/op |
1922037360 B/op |
1897382304 B/op |
1.01 |
BenchmarkRPC/attach_large_document - allocs/op |
12428 allocs/op |
12568 allocs/op |
0.99 |
BenchmarkRPC/adminCli_to_server |
544830804 ns/op 19882792 B/op 291108 allocs/op |
543085670 ns/op 20046500 B/op 291095 allocs/op |
1.00 |
BenchmarkRPC/adminCli_to_server - ns/op |
544830804 ns/op |
543085670 ns/op |
1.00 |
BenchmarkRPC/adminCli_to_server - B/op |
19882792 B/op |
20046500 B/op |
0.99 |
BenchmarkRPC/adminCli_to_server - allocs/op |
291108 allocs/op |
291095 allocs/op |
1.00 |
BenchmarkLocker |
72.56 ns/op 16 B/op 1 allocs/op |
71.89 ns/op 16 B/op 1 allocs/op |
1.01 |
BenchmarkLocker - ns/op |
72.56 ns/op |
71.89 ns/op |
1.01 |
BenchmarkLocker - B/op |
16 B/op |
16 B/op |
1 |
BenchmarkLocker - allocs/op |
1 allocs/op |
1 allocs/op |
1 |
BenchmarkLockerParallel |
40.31 ns/op 0 B/op 0 allocs/op |
39.6 ns/op 0 B/op 0 allocs/op |
1.02 |
BenchmarkLockerParallel - ns/op |
40.31 ns/op |
39.6 ns/op |
1.02 |
BenchmarkLockerParallel - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkLockerParallel - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkLockerMoreKeys |
158.3 ns/op 15 B/op 0 allocs/op |
162.6 ns/op 15 B/op 0 allocs/op |
0.97 |
BenchmarkLockerMoreKeys - ns/op |
158.3 ns/op |
162.6 ns/op |
0.97 |
BenchmarkLockerMoreKeys - B/op |
15 B/op |
15 B/op |
1 |
BenchmarkLockerMoreKeys - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkChange/Push_10_Changes |
4595026 ns/op 149331 B/op 1625 allocs/op |
4519354 ns/op 150780 B/op 1625 allocs/op |
1.02 |
BenchmarkChange/Push_10_Changes - ns/op |
4595026 ns/op |
4519354 ns/op |
1.02 |
BenchmarkChange/Push_10_Changes - B/op |
149331 B/op |
150780 B/op |
0.99 |
BenchmarkChange/Push_10_Changes - allocs/op |
1625 allocs/op |
1625 allocs/op |
1 |
BenchmarkChange/Push_100_Changes |
16658085 ns/op 782265 B/op 8513 allocs/op |
16233976 ns/op 777824 B/op 8512 allocs/op |
1.03 |
BenchmarkChange/Push_100_Changes - ns/op |
16658085 ns/op |
16233976 ns/op |
1.03 |
BenchmarkChange/Push_100_Changes - B/op |
782265 B/op |
777824 B/op |
1.01 |
BenchmarkChange/Push_100_Changes - allocs/op |
8513 allocs/op |
8512 allocs/op |
1.00 |
BenchmarkChange/Push_1000_Changes |
133052900 ns/op 7141287 B/op 79323 allocs/op |
129400417 ns/op 7207404 B/op 79325 allocs/op |
1.03 |
BenchmarkChange/Push_1000_Changes - ns/op |
133052900 ns/op |
129400417 ns/op |
1.03 |
BenchmarkChange/Push_1000_Changes - B/op |
7141287 B/op |
7207404 B/op |
0.99 |
BenchmarkChange/Push_1000_Changes - allocs/op |
79323 allocs/op |
79325 allocs/op |
1.00 |
BenchmarkChange/Pull_10_Changes |
3827212 ns/op 124040 B/op 1452 allocs/op |
3674821 ns/op 124392 B/op 1452 allocs/op |
1.04 |
BenchmarkChange/Pull_10_Changes - ns/op |
3827212 ns/op |
3674821 ns/op |
1.04 |
BenchmarkChange/Pull_10_Changes - B/op |
124040 B/op |
124392 B/op |
1.00 |
BenchmarkChange/Pull_10_Changes - allocs/op |
1452 allocs/op |
1452 allocs/op |
1 |
BenchmarkChange/Pull_100_Changes |
5606550 ns/op 353219 B/op 5178 allocs/op |
5260142 ns/op 354570 B/op 5178 allocs/op |
1.07 |
BenchmarkChange/Pull_100_Changes - ns/op |
5606550 ns/op |
5260142 ns/op |
1.07 |
BenchmarkChange/Pull_100_Changes - B/op |
353219 B/op |
354570 B/op |
1.00 |
BenchmarkChange/Pull_100_Changes - allocs/op |
5178 allocs/op |
5178 allocs/op |
1 |
BenchmarkChange/Pull_1000_Changes |
11400606 ns/op 2197724 B/op 44680 allocs/op |
10586803 ns/op 2198854 B/op 44677 allocs/op |
1.08 |
BenchmarkChange/Pull_1000_Changes - ns/op |
11400606 ns/op |
10586803 ns/op |
1.08 |
BenchmarkChange/Pull_1000_Changes - B/op |
2197724 B/op |
2198854 B/op |
1.00 |
BenchmarkChange/Pull_1000_Changes - allocs/op |
44680 allocs/op |
44677 allocs/op |
1.00 |
BenchmarkSnapshot/Push_3KB_snapshot |
19512533 ns/op 902373 B/op 8516 allocs/op |
18555858 ns/op 911370 B/op 8520 allocs/op |
1.05 |
BenchmarkSnapshot/Push_3KB_snapshot - ns/op |
19512533 ns/op |
18555858 ns/op |
1.05 |
BenchmarkSnapshot/Push_3KB_snapshot - B/op |
902373 B/op |
911370 B/op |
0.99 |
BenchmarkSnapshot/Push_3KB_snapshot - allocs/op |
8516 allocs/op |
8520 allocs/op |
1.00 |
BenchmarkSnapshot/Push_30KB_snapshot |
137275815 ns/op 8398720 B/op 92336 allocs/op |
133675736 ns/op 8206612 B/op 90703 allocs/op |
1.03 |
BenchmarkSnapshot/Push_30KB_snapshot - ns/op |
137275815 ns/op |
133675736 ns/op |
1.03 |
BenchmarkSnapshot/Push_30KB_snapshot - B/op |
8398720 B/op |
8206612 B/op |
1.02 |
BenchmarkSnapshot/Push_30KB_snapshot - allocs/op |
92336 allocs/op |
90703 allocs/op |
1.02 |
BenchmarkSnapshot/Pull_3KB_snapshot |
7924879 ns/op 1114306 B/op 20058 allocs/op |
7657503 ns/op 1116278 B/op 20060 allocs/op |
1.03 |
BenchmarkSnapshot/Pull_3KB_snapshot - ns/op |
7924879 ns/op |
7657503 ns/op |
1.03 |
BenchmarkSnapshot/Pull_3KB_snapshot - B/op |
1114306 B/op |
1116278 B/op |
1.00 |
BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op |
20058 allocs/op |
20060 allocs/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot |
20323807 ns/op 9312597 B/op 193604 allocs/op |
19600820 ns/op 9311317 B/op 193601 allocs/op |
1.04 |
BenchmarkSnapshot/Pull_30KB_snapshot - ns/op |
20323807 ns/op |
19600820 ns/op |
1.04 |
BenchmarkSnapshot/Pull_30KB_snapshot - B/op |
9312597 B/op |
9311317 B/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op |
193604 allocs/op |
193601 allocs/op |
1.00 |
BenchmarkSplayTree/stress_test_100000 |
0.2032 ns/op 0 B/op 0 allocs/op |
0.1934 ns/op 0 B/op 0 allocs/op |
1.05 |
BenchmarkSplayTree/stress_test_100000 - ns/op |
0.2032 ns/op |
0.1934 ns/op |
1.05 |
BenchmarkSplayTree/stress_test_100000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/stress_test_100000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/stress_test_200000 |
0.3923 ns/op 0 B/op 0 allocs/op |
0.402 ns/op 0 B/op 0 allocs/op |
0.98 |
BenchmarkSplayTree/stress_test_200000 - ns/op |
0.3923 ns/op |
0.402 ns/op |
0.98 |
BenchmarkSplayTree/stress_test_200000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/stress_test_200000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/stress_test_300000 |
0.5849 ns/op 0 B/op 0 allocs/op |
0.5906 ns/op 0 B/op 0 allocs/op |
0.99 |
BenchmarkSplayTree/stress_test_300000 - ns/op |
0.5849 ns/op |
0.5906 ns/op |
0.99 |
BenchmarkSplayTree/stress_test_300000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/stress_test_300000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/random_access_100000 |
0.01327 ns/op 0 B/op 0 allocs/op |
0.01253 ns/op 0 B/op 0 allocs/op |
1.06 |
BenchmarkSplayTree/random_access_100000 - ns/op |
0.01327 ns/op |
0.01253 ns/op |
1.06 |
BenchmarkSplayTree/random_access_100000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/random_access_100000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/random_access_200000 |
0.03303 ns/op 0 B/op 0 allocs/op |
0.02931 ns/op 0 B/op 0 allocs/op |
1.13 |
BenchmarkSplayTree/random_access_200000 - ns/op |
0.03303 ns/op |
0.02931 ns/op |
1.13 |
BenchmarkSplayTree/random_access_200000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/random_access_200000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/random_access_300000 |
0.04521 ns/op 0 B/op 0 allocs/op |
0.04292 ns/op 0 B/op 0 allocs/op |
1.05 |
BenchmarkSplayTree/random_access_300000 - ns/op |
0.04521 ns/op |
0.04292 ns/op |
1.05 |
BenchmarkSplayTree/random_access_300000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/random_access_300000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/editing_trace_bench |
0.002301 ns/op 0 B/op 0 allocs/op |
0.001802 ns/op 0 B/op 0 allocs/op |
1.28 |
BenchmarkSplayTree/editing_trace_bench - ns/op |
0.002301 ns/op |
0.001802 ns/op |
1.28 |
BenchmarkSplayTree/editing_trace_bench - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/editing_trace_bench - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSync/memory_sync_10_test |
7053 ns/op 1183 B/op 35 allocs/op |
6932 ns/op 1183 B/op 35 allocs/op |
1.02 |
BenchmarkSync/memory_sync_10_test - ns/op |
7053 ns/op |
6932 ns/op |
1.02 |
BenchmarkSync/memory_sync_10_test - B/op |
1183 B/op |
1183 B/op |
1 |
BenchmarkSync/memory_sync_10_test - allocs/op |
35 allocs/op |
35 allocs/op |
1 |
BenchmarkSync/memory_sync_100_test |
54168 ns/op 8531 B/op 269 allocs/op |
53455 ns/op 8544 B/op 270 allocs/op |
1.01 |
BenchmarkSync/memory_sync_100_test - ns/op |
54168 ns/op |
53455 ns/op |
1.01 |
BenchmarkSync/memory_sync_100_test - B/op |
8531 B/op |
8544 B/op |
1.00 |
BenchmarkSync/memory_sync_100_test - allocs/op |
269 allocs/op |
270 allocs/op |
1.00 |
BenchmarkSync/memory_sync_1000_test |
589540 ns/op 74609 B/op 2127 allocs/op |
547103 ns/op 76621 B/op 2256 allocs/op |
1.08 |
BenchmarkSync/memory_sync_1000_test - ns/op |
589540 ns/op |
547103 ns/op |
1.08 |
BenchmarkSync/memory_sync_1000_test - B/op |
74609 B/op |
76621 B/op |
0.97 |
BenchmarkSync/memory_sync_1000_test - allocs/op |
2127 allocs/op |
2256 allocs/op |
0.94 |
BenchmarkSync/memory_sync_10000_test |
8384276 ns/op 756341 B/op 20495 allocs/op |
7397514 ns/op 755798 B/op 20457 allocs/op |
1.13 |
BenchmarkSync/memory_sync_10000_test - ns/op |
8384276 ns/op |
7397514 ns/op |
1.13 |
BenchmarkSync/memory_sync_10000_test - B/op |
756341 B/op |
755798 B/op |
1.00 |
BenchmarkSync/memory_sync_10000_test - allocs/op |
20495 allocs/op |
20457 allocs/op |
1.00 |
BenchmarkTextEditing |
5391364253 ns/op 3922657608 B/op 20619662 allocs/op |
5283665817 ns/op 3922693896 B/op 20619778 allocs/op |
1.02 |
BenchmarkTextEditing - ns/op |
5391364253 ns/op |
5283665817 ns/op |
1.02 |
BenchmarkTextEditing - B/op |
3922657608 B/op |
3922693896 B/op |
1.00 |
BenchmarkTextEditing - allocs/op |
20619662 allocs/op |
20619778 allocs/op |
1.00 |
BenchmarkTree/10000_vertices_to_protobuf |
4516915 ns/op 6363241 B/op 70025 allocs/op |
4177030 ns/op 6363239 B/op 70025 allocs/op |
1.08 |
BenchmarkTree/10000_vertices_to_protobuf - ns/op |
4516915 ns/op |
4177030 ns/op |
1.08 |
BenchmarkTree/10000_vertices_to_protobuf - B/op |
6363241 B/op |
6363239 B/op |
1.00 |
BenchmarkTree/10000_vertices_to_protobuf - allocs/op |
70025 allocs/op |
70025 allocs/op |
1 |
BenchmarkTree/10000_vertices_from_protobuf |
223806554 ns/op 442305281 B/op 290048 allocs/op |
225664404 ns/op 442306052 B/op 290039 allocs/op |
0.99 |
BenchmarkTree/10000_vertices_from_protobuf - ns/op |
223806554 ns/op |
225664404 ns/op |
0.99 |
BenchmarkTree/10000_vertices_from_protobuf - B/op |
442305281 B/op |
442306052 B/op |
1.00 |
BenchmarkTree/10000_vertices_from_protobuf - allocs/op |
290048 allocs/op |
290039 allocs/op |
1.00 |
BenchmarkTree/20000_vertices_to_protobuf |
9471363 ns/op 12890967 B/op 140028 allocs/op |
8982727 ns/op 12890903 B/op 140028 allocs/op |
1.05 |
BenchmarkTree/20000_vertices_to_protobuf - ns/op |
9471363 ns/op |
8982727 ns/op |
1.05 |
BenchmarkTree/20000_vertices_to_protobuf - B/op |
12890967 B/op |
12890903 B/op |
1.00 |
BenchmarkTree/20000_vertices_to_protobuf - allocs/op |
140028 allocs/op |
140028 allocs/op |
1 |
BenchmarkTree/20000_vertices_from_protobuf |
894079578 ns/op 1697478504 B/op 580046 allocs/op |
882518697 ns/op 1697470072 B/op 580043 allocs/op |
1.01 |
BenchmarkTree/20000_vertices_from_protobuf - ns/op |
894079578 ns/op |
882518697 ns/op |
1.01 |
BenchmarkTree/20000_vertices_from_protobuf - B/op |
1697478504 B/op |
1697470072 B/op |
1.00 |
BenchmarkTree/20000_vertices_from_protobuf - allocs/op |
580046 allocs/op |
580043 allocs/op |
1.00 |
BenchmarkTree/30000_vertices_to_protobuf |
14471932 ns/op 18976211 B/op 210030 allocs/op |
13683783 ns/op 18976269 B/op 210029 allocs/op |
1.06 |
BenchmarkTree/30000_vertices_to_protobuf - ns/op |
14471932 ns/op |
13683783 ns/op |
1.06 |
BenchmarkTree/30000_vertices_to_protobuf - B/op |
18976211 B/op |
18976269 B/op |
1.00 |
BenchmarkTree/30000_vertices_to_protobuf - allocs/op |
210030 allocs/op |
210029 allocs/op |
1.00 |
BenchmarkTree/30000_vertices_from_protobuf |
2019314168 ns/op 3751743432 B/op 870056 allocs/op |
1966412286 ns/op 3751735888 B/op 870141 allocs/op |
1.03 |
BenchmarkTree/30000_vertices_from_protobuf - ns/op |
2019314168 ns/op |
1966412286 ns/op |
1.03 |
BenchmarkTree/30000_vertices_from_protobuf - B/op |
3751743432 B/op |
3751735888 B/op |
1.00 |
BenchmarkTree/30000_vertices_from_protobuf - allocs/op |
870056 allocs/op |
870141 allocs/op |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1142 +/- ##
==========================================
+ Coverage 38.49% 38.54% +0.05%
==========================================
Files 165 165
Lines 25169 25247 +78
==========================================
+ Hits 9688 9731 +43
- Misses 14665 14698 +33
- Partials 816 818 +2 ☔ View full report in Codecov by Sentry. |
hackerwins
force-pushed
the
refactor-retry
branch
from
45265c2 to
638932f
Compare
hackerwins
changed the title
MinWaitInterval and RequestTimeout configurations for auth webhook client
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/webhook/client.go (2)
74-118: Consider adding request validation.The
Sendmethod handles HMAC signatures and retries well, but consider adding validation for the URL parameter.Apply this diff to add URL validation:
func (c *Client[Req, Res]) Send( ctx context.Context, url, hmacKey string, body []byte, ) (*Res, int, error) { + if url == "" { + return nil, 0, fmt.Errorf("empty URL") + } signature, err := createSignature(body, hmacKey)
207-211: Consider adding documentation for status codes.The
isExpectedStatusfunction should include documentation explaining why these specific status codes are considered expected.Add this documentation:
+// isExpectedStatus returns true if the status code is one of the expected codes: +// - 200 OK: Successful request +// - 401 Unauthorized: Missing or invalid credentials +// - 403 Forbidden: Valid credentials but insufficient permissions func isExpectedStatus(statusCode int) bool {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/yorkie/server.go(3 hunks)pkg/webhook/client.go(5 hunks)pkg/webhook/client_test.go(3 hunks)server/backend/backend.go(4 hunks)server/config.go(2 hunks)server/config.sample.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/config.sample.yml
- cmd/yorkie/server.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: bench
🔇 Additional comments (15)
server/config.go (3)
63-66: Verify the impact of reduced request timeout.The default request timeout has been significantly reduced from 30 seconds to 3 seconds. While this could improve performance by failing faster, we should ensure this doesn't increase failure rates in production environments with higher latency.
Could you share the reasoning behind this significant reduction and confirm if this has been tested in environments with varying network conditions?
190-197: LGTM! Implementation follows established patterns.The default value handling for the new configurations follows the consistent pattern used throughout the file, maintaining code style and proper string conversion for durations.
113-137: Verify validation implementation for new fields.The
Validate()method delegates validation to the backend config, but we don't see the validation implementation for the new duration fields. Please ensure proper validation is implemented in the backend package to verify these are valid duration strings.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for validation of the new fields rg -A 5 "func.*Validate" server/backend/Length of output: 1343
Validation checks for new duration fields confirmed.
server/backend/database/mongo/config.govalidatesConnectionTimeoutusingtime.ParseDuration().server/backend/housekeeping/config.govalidates theIntervalfield.server/backend/config.govalidates theClientDeactivateThreshold.No issues with missing or incorrect validations were found.
server/backend/backend.go (3)
52-53: LGTM! Well-documented field addition.The new
WebhookClientfield is properly documented and follows the same pattern as other fields in the struct.
160-161: LGTM! Proper initialization order.The initialization order in the struct literal is correct, with related fields (
WebhookCacheandWebhookClient) grouped together.
89-100: Verify the initialization sequence.The initialization sequence looks good, but let's verify that the order of operations is correct and that all necessary error handling is in place.
Run the following script to verify the initialization sequence:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify the initialization sequence and error handling. # Test: Search for initialization patterns in the codebase. rg -A 5 'New\(' server/backend/Length of output: 14840
Initialization sequence verified
After reviewing the code and comparing it to similar patterns in the codebase, the webhook cache and client are instantiated in the correct order. Their respective constructor functions (i.e.
cache.NewLRUExpireCacheandwebhook.NewClient) do not return errors, so no additional error handling is needed here.
- The webhook cache and client are initialized after obtaining the proper configuration.
- Their usage is consistent with the overall initialization sequence in
server/backend/backend.go.pkg/webhook/client.go (4)
48-54: LGTM! Well-structured options.The
Optionsstruct is well-organized with clear field names and appropriate types.
120-138: LGTM! Clean request building.The
buildRequestmethod is well-structured with proper error handling and header setting.
140-151: LGTM! Secure signature creation.The
createSignaturefunction properly handles HMAC signature creation with good error handling.
182-190: LGTM! Proper interval calculation.The
waitIntervalfunction correctly implements exponential backoff with bounds checking.pkg/webhook/client_test.go (5)
87-99: Optimize test execution time.The
newDelayServerimplementation can be improved to avoid actual delays during testing.Apply this diff to use a more efficient approach:
func newDelayServer(t *testing.T, delayTime time.Duration, responseData testResponse) *httptest.Server { return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx, cancel := context.WithTimeout(r.Context(), delayTime) - defer cancel() - - select { - case <-ctx.Done(): - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - assert.NoError(t, json.NewEncoder(w).Encode(responseData)) - } + timer := time.NewTimer(0) + timer.Reset(delayTime) + <-timer.C + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + assert.NoError(t, json.NewEncoder(w).Encode(responseData)) })) }
100-162: LGTM! Comprehensive HMAC tests.The HMAC tests cover all important scenarios: valid key, invalid key, missing key, and empty body.
164-209: LGTM! Well-structured backoff tests.The backoff tests properly verify both successful and failed retry scenarios.
211-253: LGTM! Thorough timeout tests.The timeout tests effectively verify both successful and failed timeout scenarios.
255-290: LGTM! Comprehensive error handling tests.The error handling tests cover important scenarios like context cancellation and invalid URLs.
What this PR does / why we need it:
This PR adds
MinWaitIntervalandRequestTimeoutconfigurations for the auth webhook client to enhance control over the timing and reliability of authentication requests.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Previously:
MinWaitIntervalwas set to100 * time.MillisecondRequestTimeoutwas30 * time.Second, usinghttp.Post()withhttp.DefaultClientTo improve performance and prevent authentication timeouts, this PR establishes default configurations:
Please review whether these default values are appropriate.
Does this PR introduce a user-facing change?:
Additional documentation:
There are four configuration options related to timeouts, which can be confusing. Below is a brief explanation:
Checklist:
Summary by CodeRabbit
New Features
Documentation