Backport PR #2565 to release/v1.7 for [BUGFIX] index correction process #2566
Conversation
* [BUGFIX] index correction process Signed-off-by: kpango <[email protected]> * fix: disable chaos deploy Signed-off-by: hlts2 <[email protected]> * Revert "fix: disable chaos deploy" This reverts commit 48aa7a9. * fix: add condition for k3d ingress Signed-off-by: hlts2 <[email protected]> --------- Signed-off-by: kpango <[email protected]> Signed-off-by: hlts2 <[email protected]> Co-authored-by: hlts2 <[email protected]>
Deploying vald with
|
| Latest commit: |
c3a46df
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e98e37ea.vald.pages.dev |
| Branch Preview URL: | https://backport-release-v1-7-bugfix-5m1q.vald.pages.dev |
|
Caution Review failedThe pull request is closed. Walkthrough
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
[CHATOPS:HELP] ChatOps commands.
|
|
[WARNING:INTCFG] Changes in |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (9)
dockers/binfmt/Dockerfile (1)
1-1: Add comments for clarity.Consider adding comments to explain the purpose of each section of the Dockerfile for better readability and maintainability.
internal/config/corrector_test.go (2)
61-65: Ensure correct initialization of new fields in test cases.The fields
KVSBackgroundSyncInterval,KVSBackgroundCompactionInterval, andGatewayare present in the test cases withininternal/config/corrector_test.gobut are initialized with empty values. Ensure that these fields are initialized with meaningful values in the test cases to properly test their functionality.
internal/config/corrector_test.go: Lines where fields are initialized with empty values.Analysis chain
Ensure correct initialization of new fields in test cases.
The new fields
KVSBackgroundSyncInterval,KVSBackgroundCompactionInterval, andGatewayshould be correctly initialized in the test cases.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct initialization of new fields in test cases. # Test: Search for the initialization of new fields in test cases. Expect: Proper initialization. rg --type go -A 5 $'KVSBackgroundSyncInterval|KVSBackgroundCompactionInterval|Gateway'Length of output: 107923
Script:
#!/bin/bash # Description: Verify the correct initialization of new fields in test cases within internal/config/corrector_test.go. # Test: Search for the initialization of KVSBackgroundSyncInterval, KVSBackgroundCompactionInterval, and Gateway fields in internal/config/corrector_test.go. rg --type go -A 5 'KVSBackgroundSyncInterval|KVSBackgroundCompactionInterval|Gateway' internal/config/corrector_test.goLength of output: 1682
90-94: Ensure correct initialization of new fields in additional test cases.The new fields
KVSBackgroundSyncInterval,KVSBackgroundCompactionInterval, andGatewayare being initialized to empty strings or default values in the test cases. Please ensure these fields are initialized with meaningful values to properly test their functionality.
internal/config/corrector_test.go: Lines where the fields are initialized.Analysis chain
Ensure correct initialization of new fields in additional test cases.
The new fields
KVSBackgroundSyncInterval,KVSBackgroundCompactionInterval, andGatewayshould be correctly initialized in additional test cases.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct initialization of new fields in additional test cases. # Test: Search for the initialization of new fields in additional test cases. Expect: Proper initialization. rg --type go -A 5 $'KVSBackgroundSyncInterval|KVSBackgroundCompactionInterval|Gateway'Length of output: 107923
Script:
#!/bin/bash # Description: Verify the correct initialization of new fields in additional test cases in internal/config/corrector_test.go. # Test: Search for the initialization of new fields in the corrector_test.go file. Expect: Proper initialization. rg --type go -A 5 'KVSBackgroundSyncInterval|KVSBackgroundCompactionInterval|Gateway' internal/config/corrector_test.goLength of output: 1682
Script:
#!/bin/bash # Description: Extract more context around the initialization of new fields in the corrector_test.go file. # Test: Search for the initialization of new fields in the corrector_test.go file with more context lines. Expect: Proper initialization. rg --type go -A 10 'KVSBackgroundSyncInterval|KVSBackgroundCompactionInterval|Gateway' internal/config/corrector_test.goLength of output: 2100
pkg/index/job/correction/service/options_test.go (5)
Line range hint
102-272: Implement the test cases forTestWithIndexReplica.The function structure is well-defined, but the test cases need to be implemented.
Do you want me to implement the test cases or open a GitHub issue to track this task?
Line range hint
273-442: Implement the test cases forTestWithDiscoverer.The function structure is well-defined, but the test cases need to be implemented.
Do you want me to implement the test cases or open a GitHub issue to track this task?
Line range hint
443-526: Implement the test cases forTestWithGateway.The function structure is well-defined, but the test cases need to be implemented.
Do you want me to implement the test cases or open a GitHub issue to track this task?
Line range hint
527-604: Implement the test cases forTestWithKVSSyncInterval.The function structure is well-defined, but the test cases need to be implemented.
Do you want me to implement the test cases or open a GitHub issue to track this task?
Line range hint
605-686: Implement the test cases forTestWithKVSCompactionInterval.The function structure is well-defined, but the test cases need to be implemented.
Do you want me to implement the test cases or open a GitHub issue to track this task?
internal/db/kvs/pogreb/pogreb_test.go (1)
Line range hint
605-686: Implement the test cases forTest_db_SetandTest_db_Close.The function structures are well-defined, but the test cases need to be implemented.
Do you want me to implement the test cases or open a GitHub issue to track this task?
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
example/client/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumrust/Cargo.lockis excluded by!**/*.lock
Files selected for processing (54)
- .gitfiles (3 hunks)
- .github/actions/setup-k3d/action.yaml (1 hunks)
- .github/helm/values/values-correction.yaml (1 hunks)
- .github/workflows/dockers-binfmt-image.yaml (1 hunks)
- .github/workflows/dockers-buildkit-image.yaml (1 hunks)
- Makefile (2 hunks)
- Makefile.d/dependencies.mk (1 hunks)
- Makefile.d/docker.mk (2 hunks)
- Makefile.d/e2e.mk (1 hunks)
- Makefile.d/functions.mk (1 hunks)
- Makefile.d/k8s.mk (2 hunks)
- Makefile.d/proto.mk (2 hunks)
- Makefile.d/test.mk (1 hunks)
- charts/vald-helm-operator/crds/valdrelease.yaml (2 hunks)
- charts/vald/README.md (1 hunks)
- charts/vald/templates/gateway/lb/networkpolicy.yaml (2 hunks)
- charts/vald/templates/index/job/correction/configmap.yaml (1 hunks)
- charts/vald/templates/index/job/correction/networkpolicy.yaml (2 hunks)
- charts/vald/values.schema.json (2 hunks)
- charts/vald/values.yaml (1 hunks)
- dockers/binfmt/Dockerfile (1 hunks)
- dockers/buildkit/Dockerfile (1 hunks)
- dockers/ci/base/Dockerfile (1 hunks)
- example/client/go.mod (2 hunks)
- go.mod (9 hunks)
- internal/config/corrector.go (2 hunks)
- internal/config/corrector_test.go (4 hunks)
- internal/db/kvs/pogreb/options.go (2 hunks)
- internal/db/kvs/pogreb/options_test.go (2 hunks)
- internal/db/kvs/pogreb/pogreb.go (2 hunks)
- internal/db/kvs/pogreb/pogreb_test.go (16 hunks)
- internal/errors/corrector.go (1 hunks)
- internal/servers/servers.go (2 hunks)
- k8s/index/job/correction/configmap.yaml (1 hunks)
- k8s/index/job/correction/cronjob.yaml (1 hunks)
- k8s/index/job/creation/configmap.yaml (1 hunks)
- k8s/index/job/creation/cronjob.yaml (1 hunks)
- k8s/index/job/save/configmap.yaml (1 hunks)
- k8s/index/job/save/cronjob.yaml (1 hunks)
- k8s/index/operator/configmap.yaml (1 hunks)
- k8s/index/operator/deployment.yaml (1 hunks)
- k8s/index/operator/priorityclass.yaml (1 hunks)
- k8s/operator/helm/crds/valdrelease.yaml (2 hunks)
- pkg/agent/core/ngt/service/ngt_test.go (3 hunks)
- pkg/gateway/lb/handler/grpc/handler_test.go (3 hunks)
- pkg/index/job/correction/service/corrector.go (3 hunks)
- pkg/index/job/correction/service/corrector_test.go (26 hunks)
- pkg/index/job/correction/service/options.go (3 hunks)
- pkg/index/job/correction/service/options_test.go (6 hunks)
- pkg/index/job/correction/usecase/corrector.go (5 hunks)
- tests/e2e/crud/crud_test.go (1 hunks)
- versions/CMAKE_VERSION (1 hunks)
- versions/PROMETHEUS_STACK_VERSION (1 hunks)
- versions/YQ_VERSION (1 hunks)
Files not processed due to max files limit (1)
- versions/actions/ACTIONS_UPLOAD_ARTIFACT
Files skipped from review due to trivial changes (7)
- .gitfiles
- dockers/buildkit/Dockerfile
- dockers/ci/base/Dockerfile
- pkg/agent/core/ngt/service/ngt_test.go
- versions/CMAKE_VERSION
- versions/PROMETHEUS_STACK_VERSION
- versions/YQ_VERSION
Additional comments not posted (168)
charts/vald/README.md (2)
Line range hint
1-1:
Add documentation for new parameters.Ensure that the new parameters
gateway,kvs_background_compaction_interval, andkvs_background_sync_intervalare clearly documented with examples.
Line range hint
1-1:
Verify removal ofkvs_async_write_concurrency.Ensure that the removal of the
kvs_async_write_concurrencyparameter does not affect existing functionality or performance.charts/vald/values.schema.json (2)
Line range hint
1-1:
Add documentation for newgatewayobject.Ensure that the new
gatewayobject and its properties are clearly documented with examples.
Line range hint
1-1:
Verify replacement ofkvs_async_write_concurrency.Ensure that the new properties
kvs_background_compaction_intervalandkvs_background_sync_intervalcorrectly replace the functionality of the removedkvs_async_write_concurrency.dockers/binfmt/Dockerfile (1)
1-1: Ensure Dockerfile follows best practices.Verify that the Dockerfile follows best practices for security, efficiency, and maintainability.
k8s/index/operator/priorityclass.yaml (1)
1-30: LGTM!The
PriorityClassconfiguration is correct and follows Kubernetes best practices..github/workflows/dockers-binfmt-image.yaml (1)
1-48: LGTM!The GitHub Actions workflow for building the
binfmtDocker image is correct and follows best practices..github/workflows/dockers-buildkit-image.yaml (1)
1-48: LGTM!The GitHub Actions workflow for building the
buildkitDocker image is correct and follows best practices.internal/db/kvs/pogreb/options.go (2)
43-50: LGTM! But verify the function usage in the codebase.The change to accept a
time.Durationinstead of astringimproves type safety and simplifies the function. Ensure that all references to this function in the codebase are updated accordingly.Verification successful
LGTM! The function usage has been updated correctly.
The change to accept a
time.Durationinstead of astringimproves type safety and simplifies the function. All references to this function in the codebase have been updated accordingly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `WithBackgroundSyncInterval` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'WithBackgroundSyncInterval'Length of output: 5663
55-62: LGTM! But verify the function usage in the codebase.The change to accept a
time.Durationinstead of astringimproves type safety and simplifies the function. Ensure that all references to this function in the codebase are updated accordingly.Verification successful
Verify the function usage in
pkg/index/job/correction/service/corrector.goto ensure it matches the new signature.The function
WithBackgroundCompactionIntervalis used inpkg/index/job/correction/service/corrector.go. Ensure that it passes atime.Durationargument to match the updated function signature.
Verification successful.
The function
WithBackgroundCompactionIntervalis correctly used with atime.Durationargument inpkg/index/job/correction/service/corrector.go, matching the updated function signature.
pkg/index/job/correction/service/corrector.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `WithBackgroundCompactionInterval` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'WithBackgroundCompactionInterval'Length of output: 2034
Script:
#!/bin/bash # Description: Verify the argument type for `WithBackgroundCompactionInterval` in `pkg/index/job/correction/service/corrector.go`. # Search for the definition of `backgroundCompactionInterval` to ensure it is of type `time.Duration`. rg --type go 'backgroundCompactionInterval'Length of output: 2921
example/client/go.mod (5)
15-15: Dependency Update:google.golang.org/genproto/googleapis/apiThe version has been updated from
v0.0.0-20240730163845-b1a4ccb954bftov0.0.0-20240805194559-2c9e96a0b5d4. Ensure that the updated version is compatible with the rest of the codebase.
6-6: Dependency Update:github.com/envoyproxy/protoc-gen-validateThe version has been updated from
v1.0.4tov1.1.0. Ensure that the updated version is compatible with the rest of the codebase.
40-40: ```shell
#!/bin/bashDescription: Verify compatibility of the updated dependency
google.golang.org/genproto/googleapis/rpcin the identified files.Test: Inspect usage in
tests/e2e/performance/max_vector_dim_test.goecho "Inspecting tests/e2e/performance/max_vector_dim_test.go"
rg --type go -A 10 'google.golang.org/genproto/googleapis/rpc' tests/e2e/performance/max_vector_dim_test.goTest: Inspect usage in
internal/net/grpc/status/status.goecho "Inspecting internal/net/grpc/status/status.go"
rg --type go -A 10 'google.golang.org/genproto/googleapis/rpc' internal/net/grpc/status/status.goTest: Inspect usage in
internal/net/grpc/errdetails/errdetails.goecho "Inspecting internal/net/grpc/errdetails/errdetails.go"
rg --type go -A 10 'google.golang.org/genproto/googleapis/rpc' internal/net/grpc/errdetails/errdetails.goTest: Inspect usage in
apis/grpc/v1/payload/payload_vtproto.pb.goecho "Inspecting apis/grpc/v1/payload/payload_vtproto.pb.go"
rg --type go -A 10 'google.golang.org/genproto/googleapis/rpc' apis/grpc/v1/payload/payload_vtproto.pb.goTest: Inspect usage in
apis/grpc/v1/payload/payload.pb.goecho "Inspecting apis/grpc/v1/payload/payload.pb.go"
rg --type go -A 10 'google.golang.org/genproto/googleapis/rpc' apis/grpc/v1/payload/payload.pb.go--- `14-14`: ```shell #!/bin/bash # Description: List the tags or commit messages in the `google.golang.org/genproto` repository to find the correct versions. # Fetch the tags or commit messages for `google.golang.org/genproto` curl -s https://api.github.com/repos/googleapis/go-genproto/tags | jq '.[] | {name: .name, commit: .commit.sha}'
16-16: Dependency Update:google.golang.org/genproto/googleapis/rpcThe version has been updated from
v0.0.0-20240730163845-b1a4ccb954bftov0.0.0-20240805194559-2c9e96a0b5d4. Ensure that the updated version is compatible with the rest of the codebase..github/helm/values/values-correction.yaml (6)
17-21: Defaults Configuration: Logging and Network PolicyThe logging level is set to
infoand the network policy is enabled. These configurations are standard and necessary for the application.
22-33: Gateway Configuration: Load Balancing and ResourcesThe load balancing is enabled with minimum replicas set to 1. The resource requests are set to reasonable values. The gateway configuration includes an index replica setting. These configurations are standard and necessary for the application.
34-48: Agent Configuration: Replicas, Resources, and NGT SettingsThe agent configuration includes settings for minimum and maximum replicas, resource requests, and NGT-specific settings such as auto-index duration limit and dimension. These configurations are standard and necessary for the application.
49-56: Discoverer Configuration: Replicas and ResourcesThe discoverer configuration includes settings for minimum replicas and resource requests. These configurations are standard and necessary for the application.
57-67: Manager Configuration: Index Replicas, Resources, and Indexer SettingsThe manager configuration includes settings for index replicas, resource requests, and indexer-specific settings such as auto-index duration limit and auto-index length. These configurations are standard and necessary for the application.
68-73: Corrector Configuration: Enabling, Suspending for CI, and ScheduleThe corrector configuration includes settings for enabling the corrector, suspending it for CI, and setting a schedule. These configurations are standard and necessary for the application.
charts/vald/templates/index/job/correction/networkpolicy.yaml (2)
18-18: LGTM!The new variable
$lbdeclaration for the load balancer configuration is correct and aligns with Helm templating practices.
55-60: LGTM!The new rules under the
specsection enhance the template's flexibility in managing network policies and provide better integration with the load balancer's settings.internal/errors/corrector.go (1)
29-30: LGTM!The new error variable
ErrNoAvailableAgentToRemoveenhances the error handling capabilities by providing a specific error message for the scenario where there is no available agent to remove a replica.charts/vald/templates/gateway/lb/networkpolicy.yaml (2)
22-22: LGTM!The new variable
$correctordeclaration for the corrector configuration is correct and aligns with Helm templating practices.
52-57: LGTM!The new ingress rules enhance the template's flexibility in managing network policies and provide better integration with the corrector's settings.
Makefile.d/proto.mk (2)
50-50: LGTM! Improved maintainability by using a variable instead of a hardcoded path.The change enhances flexibility and makes future updates easier.
62-65: LGTM! Enhanced maintainability by using variables instead of hardcoded paths.The changes improve readability and make future updates easier.
internal/config/corrector.go (3)
41-45: LGTM! Added new fields for KVS operation intervals.The changes enhance configuration flexibility for KVS operations.
53-54: LGTM! Added a new field for gateway service configuration.The change extends the service configuration capabilities.
63-71: LGTM! Updated the Bind method to process new fields.The changes ensure proper handling of the new configuration options.
internal/servers/servers.go (1)
23-23: LGTM! Replaced thesortpackage with theslicespackage for better efficiency.The changes streamline slice operations and improve code clarity.
Also applies to: 53-54
pkg/index/job/correction/service/options.go (4)
33-40: LGTM!The function correctly checks for a nil error group before assigning it.
65-74: LGTM!The function correctly checks for a nil gateway client before assigning it.
87-100: LGTM!The function correctly parses the duration string and assigns it to the configuration.
102-113: LGTM!The function correctly parses the duration string and assigns it to the configuration.
internal/db/kvs/pogreb/pogreb.go (2)
27-29: LGTM!Renaming the interface to
DBreflects a more generic usage, which is a good practice for standardizing database interactions.
46-47: LGTM!Updating the return type to
DBaligns with the new naming convention and reinforces the idea of a more generalized database interface..github/actions/setup-k3d/action.yaml (1)
84-95: LGTM!The new logic improves flexibility and ensures consistent application of options regardless of the initial state of
K3D_OPTIONS.internal/config/corrector_test.go (2)
20-30: Ensure new fields are correctly integrated.The new fields
KVSBackgroundSyncInterval,KVSBackgroundCompactionInterval, andGatewayhave been added to thefieldsstruct. Ensure that these fields are correctly initialized and utilized in the test cases.
125-135: Ensure correct assignment of new fields in theCorrectorstruct.The new fields
KVSBackgroundSyncInterval,KVSBackgroundCompactionInterval, andGatewayshould be correctly assigned in theCorrectorstruct.Verification successful
Ensure correct assignment of new fields in the
Correctorstruct.The fields
KVSBackgroundSyncInterval,KVSBackgroundCompactionInterval, andGatewayare correctly assigned in theCorrectorstruct. The assignments are also covered in the test cases ininternal/config/corrector_test.go.
KVSBackgroundSyncIntervalis assigned ininternal/config/corrector.goand tested ininternal/config/corrector_test.go.KVSBackgroundCompactionIntervalis assigned ininternal/config/corrector.goand tested ininternal/config/corrector_test.go.Gatewayis assigned ininternal/config/corrector.goand tested ininternal/config/corrector_test.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct assignment of new fields in the `Corrector` struct. # Test: Search for the assignment of new fields in the `Corrector` struct. Expect: Proper assignment. rg --type go -A 5 $'KVSBackgroundSyncInterval|KVSBackgroundCompactionInterval|Gateway'Length of output: 107923
charts/vald/templates/index/job/correction/configmap.yaml (2)
54-55: Ensure correct integration of new KVS parameters.The new parameters
kvs_background_sync_intervalandkvs_background_compaction_intervalreplace the existingkvs_async_write_concurrencyparameter. Ensure that these new parameters are correctly integrated and align with the PR objectives.Verification successful
The new KVS parameters are correctly integrated.
The parameters
kvs_background_sync_intervalandkvs_background_compaction_intervalhave been correctly integrated across the relevant configuration and CRD files. They are defined incharts/vald/values.yaml, referenced incharts/vald/templates/index/job/correction/configmap.yaml, and included in the CRD schemas.
- Files reviewed:
k8s/operator/helm/crds/valdrelease.yamlk8s/index/job/correction/configmap.yamlcharts/vald/values.yamlcharts/vald-helm-operator/crds/valdrelease.yamlcharts/vald/templates/index/job/correction/configmap.yamlScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct integration of new KVS parameters. # Test: Search for the usage of new KVS parameters. Expect: Proper integration. rg --type yaml -A 5 $'kvs_background_sync_interval|kvs_background_compaction_interval'Length of output: 3993
57-75: ```shell
#!/bin/bashDescription: Extract the relevant context around the
gatewaykey and the conditional logic for$gateway.enabledin the file under review.Extract 10 lines before and after the conditional logic for
$gateway.enabledin the specified file.rg --type yaml -A 10 -B 10 '{{- if $gateway.enabled -}}' charts/vald/templates/index/job/correction/configmap.yaml
</blockquote></details> <details> <summary>k8s/index/job/save/cronjob.yaml (3)</summary><blockquote> `16-32`: **Ensure correct implementation of new CronJob configuration.** The new Kubernetes CronJob configuration specifies a job that runs every three hours, with a policy to forbid concurrent executions. Ensure that the new CronJob configuration is correctly implemented and aligns with the PR objectives. ```shell #!/bin/bash # Description: Verify the correct implementation of new CronJob configuration. # Test: Search for the new CronJob configuration. Expect: Proper implementation. rg --type yaml -A 5 $'apiVersion: batch/v1|kind: CronJob'
53-76: Ensure correct implementation of initialization containers for readiness checks.The job template includes initialization containers that check the readiness of dependent services before the main application container starts. Ensure that the initialization containers are correctly implemented and align with the PR objectives.
#!/bin/bash # Description: Verify the correct implementation of initialization containers for readiness checks. # Test: Search for the initialization containers for readiness checks. Expect: Proper implementation. rg --type yaml -A 5 $'initContainers:|name: wait-for-agent|name: wait-for-discoverer'
77-125: Ensure correct implementation of main container configuration.The main container,
vald-index-save, is configured with several probes (liveness, readiness, and startup) to monitor its health and ensure it is functioning correctly. Ensure that the main container configuration is correctly implemented and aligns with the PR objectives.#!/bin/bash # Description: Verify the correct implementation of main container configuration. # Test: Search for the main container configuration. Expect: Proper implementation. rg --type yaml -A 5 $'containers:|name: vald-index-save|livenessProbe:|readinessProbe:|startupProbe:'k8s/index/job/creation/cronjob.yaml (5)
16-27: Metadata section looks good.The metadata section is well-configured with appropriate labels and Helm chart information.
28-32: Spec section looks good.The spec section is well-configured with appropriate schedule, concurrency policy, and starting deadline seconds.
33-51: Job template metadata section looks good.The job template metadata section is well-configured with appropriate labels and annotations for monitoring.
52-75: InitContainers section looks good.The initContainers section is well-configured with appropriate readiness checks for
vald-agentandvald-discoverer.
76-144: Containers section looks good.The containers section is well-configured with appropriate probes, ports, environment variables, and volume mounts.
k8s/index/job/correction/cronjob.yaml (5)
16-27: Metadata section looks good.The metadata section is well-configured with appropriate labels and Helm chart information.
28-32: Spec section looks good.The spec section is well-configured with appropriate schedule, concurrency policy, and starting deadline seconds.
33-51: Job template metadata section looks good.The job template metadata section is well-configured with appropriate labels and annotations for monitoring.
52-75: InitContainers section looks good.The initContainers section is well-configured with appropriate readiness checks for
vald-agentandvald-discoverer.
76-144: Containers section looks good.The containers section is well-configured with appropriate probes, ports, environment variables, and volume mounts.
k8s/index/operator/deployment.yaml (5)
16-27: Metadata section looks good.The metadata section is well-configured with appropriate labels and Helm chart information.
28-39: Spec section looks good.The spec section is well-configured with appropriate replicas, update strategy, and pod template specifications.
40-54: Pod template metadata section looks good.The pod template metadata section is well-configured with appropriate labels and annotations for monitoring.
55-172: Containers section looks good.The containers section is well-configured with appropriate probes, ports, environment variables, and volume mounts.
173-173: Status section looks good.The status section is empty as expected.
pkg/index/job/correction/usecase/corrector.go (5)
19-19: LGTM! Importing theslicespackage.The import of the
slicespackage is appropriate for reversing slices, improving readability and potentially performance.
52-57: LGTM! Retrieving and appending gateway options with error group handling.The changes ensure that errors during concurrent operations are managed appropriately.
59-62: LGTM! Initializing the new gateway client.The initialization of the new gateway client enhances the corrector's capabilities in managing client interactions.
89-90: LGTM! Usingslices.Reverseto reverse the address list.The use of
slices.Reverseimproves readability and potentially performance compared to manually reversing the list.
117-121: LGTM! Adding parameters for KVS synchronization and compaction intervals.The addition of these parameters indicates a more robust configuration management for the corrector service.
Makefile.d/e2e.mk (1)
133-133: LGTM! Updating the Helm values file path.The update from
values-lb.yamltovalues-correction.yamlsuggests an adjustment in the configuration parameters for the deployment, which may affect the behavior or settings of the application being deployed.Makefile.d/docker.mk (3)
19-41: LGTM!The restructuring of the
docker/buildtarget enhances the modularity and maintainability of the build process.
200-210: LGTM!The addition of
.PHONYtargets fordocker/name/buildkitanddocker/build/buildkitenhances the flexibility and clarity of the build process.
211-221: LGTM!The addition of
.PHONYtargets fordocker/name/binfmtanddocker/build/binfmtenhances the flexibility and clarity of the build process.Makefile.d/dependencies.mk (2)
228-228: LGTM!The change to use the
$(REPO)variable in the curl command enhances the flexibility of the Makefile.
233-233: LGTM!The change to use the
$(REPO)variable in the curl command enhances the flexibility of the Makefile.k8s/index/job/save/configmap.yaml (3)
18-27: LGTM!The metadata and labels for the ConfigMap are correctly defined.
36-165: LGTM!The server configuration is comprehensive and well-structured, including settings for gRPC, health check servers, and metrics servers.
173-370: LGTM!The observability and saver configuration is detailed and enhances the operational parameters of the
vald-index-savecomponent.k8s/index/job/creation/configmap.yaml (6)
1-25: LGTM!The header and metadata section are correctly defined.
27-35: LGTM!The version, time zone, and logging configurations are appropriately defined.
36-78: LGTM!The server configurations for gRPC and REST are appropriately defined.
79-155: LGTM!The health check and metrics server configurations are appropriately defined.
156-165: LGTM!The startup and shutdown strategies are appropriately defined.
167-370: LGTM!The TLS, observability, and creator configurations are appropriately defined.
Makefile.d/test.mk (1)
Line range hint
347-364:
LGTM!The modifications to replace hardcoded import paths with variable references are appropriately defined.
Makefile.d/functions.mk (1)
121-135: LGTM!The modifications to the
run-e2e-crud-testcommand, adding new parameters for granular control over data operations, are appropriately defined.k8s/index/job/correction/configmap.yaml (6)
1-25: LGTM!The licensing and metadata section follows standard practices and is correctly configured.
36-155: Verify default values for timeouts and buffer sizes.Ensure that the default values for timeouts, buffer sizes, and other parameters meet performance and security requirements.
156-165: LGTM!The startup and shutdown strategies section is well-defined and follows best practices.
167-172: Verify paths to certificates and keys.Ensure that the paths to the certificates and keys are correct and accessible.
173-204: Verify OTLP collector endpoints and metrics settings.Ensure that the OTLP collector endpoints and metrics settings are correctly configured to enhance observability.
205-451: Verify values for connection pooling, circuit breaker settings, and other parameters.Ensure that the values for connection pooling, circuit breaker settings, and other parameters meet performance and reliability requirements.
internal/db/kvs/pogreb/pogreb_test.go (5)
54-62: LGTM!The changes to the
WithBackgroundSyncIntervaloption simplify the parameter type and align it with a more consistent usage pattern.
125-126: LGTM!The changes to the
beforeFuncandafterFuncfunction signatures enhance flexibility and potentially improve test coverage.Also applies to: 159-165, 165-165
250-252: LGTM!The changes to the
checkFunc,beforeFunc, andafterFuncfunction signatures enhance flexibility and potentially improve test coverage.Also applies to: 254-254, 271-275, 275-288, 288-294, 294-294
349-350: LGTM!The changes to the
beforeFuncandafterFuncfunction signatures enhance flexibility and potentially improve test coverage.Also applies to: 387-395, 395-395
493-494: LGTM!The changes to the
beforeFuncandafterFuncfunction signatures enhance flexibility and potentially improve test coverage.Also applies to: 519-527, 527-527
Makefile.d/k8s.mk (6)
40-43: Verify the impact of enabling new features.The new settings enable the operator, saver, creator, and corrector features for the manager index component. Ensure that these changes are tested thoroughly to verify their impact on the deployment process.
543-543: LGTM!The use of the
$(REPO)variable enhances flexibility by dynamically constructing the deployment name.
548-548: LGTM!The use of the
$(REPO)variable enhances flexibility by dynamically constructing the deployment name.
553-553: LGTM!The use of the
$(REPO)variable enhances flexibility by dynamically constructing the deployment name.
558-558: LGTM!The use of the
$(REPO)variable enhances flexibility by dynamically constructing the deployment name.
563-563: LGTM!The use of the
$(REPO)variable enhances flexibility by dynamically constructing the deployment name.tests/e2e/crud/crud_test.go (1)
80-80: Verify the impact of increasingcorrectionInsertNum.The value for the
correctionInsertNumflag has been increased from 3000 to 10000. Ensure that this change is tested thoroughly to verify its impact on test performance and resource usage.k8s/index/operator/configmap.yaml (3)
1-15: LGTM!The license header is standard and does not require any changes.
16-27: LGTM!The metadata for the ConfigMap is well-structured and follows Kubernetes conventions.
28-28: Verify the comprehensive configuration in a staging environment.The
datasection includes a comprehensive configuration for server and health check settings. Ensure that these settings are tested thoroughly in a staging environment to verify their correctness and impact on the application.Makefile (4)
20-21: Good use of centralizing repository structure.The introduction of the
REPOvariable enhances the clarity and maintainability of theMakefile.
25-25: Maintains consistency.Updating the
GHCRORGvariable to use theREPOvariable maintains consistency across the file.
26-34: Enhanced organization and scope expansion.The introduction and reorganization of image variables reflect an expansion in the scope of images managed by the
Makefileand enhance the organization.
357-363: Facilitates granular control over E2E testing.The addition of new environment variables for E2E testing enhances the testing framework's capabilities by facilitating more granular control over the testing parameters.
pkg/index/job/correction/service/corrector.go (8)
30-31: Necessary imports for new functionality.The new imports for
vcandpogrebare necessary for the newgatewayclient and the transition topogreb.
47-50: Improves observability and monitoring.Adding methods for metrics to the
Correctorinterface improves observability and monitoring of theCorrectorfunctionality.
55-67: Enhances functionality and maintainability.The updates to the
correctstruct, including new fields and the removal of old ones, reflect the transition topogreband the introduction of thegatewayclient, enhancing its functionality and maintainability.
83-98: Crucial initialization for new functionality.The
Newfunction has been updated to initialize thepogrebdatabase, and error handling has been appropriately implemented.
102-130: Enhances robustness with improved error handling.The
StartClientmethod has been updated to initiate the newgatewayclient and handle errors more effectively using anerrgroup, enhancing its robustness.
133-691: Improves clarity, error handling, and robustness.The
Startmethod and associated logic have been significantly updated to align with the new architecture, improving the clarity of the data flow and enhancing error handling, logging, and the overall robustness of the correction process.
697-699: Ensures data integrity and resource cleanup.The
PreStopmethod has been updated to close thepogrebdatabase properly, ensuring data integrity and resource cleanup.
702-711: Improves observability and monitoring.Adding methods for metrics to the
correctstruct improves observability and monitoring of thecorrectfunctionality.pkg/index/job/correction/service/corrector_test.go (13)
112-122: Shift towards integrated or simplified dependency management.The removal of the
mockDiscovererClientstruct and its associated methods suggests a shift in how dependencies are managed within the tests, possibly moving towards a more integrated or simplified approach.
155-165: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
187-197: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
228-238: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
254-264: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
293-303: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
325-335: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
366-376: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
392-402: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
431-441: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
463-473: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
504-514: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.
527-537: Ensures consistency in tests.The updates to the
correctstruct's fields in the test cases align with the changes in the maincorrectstruct, ensuring consistency in the tests.go.mod (10)
319-321: Verify compatibility withgoogle.golang.org/genprotov0.0.0-20240805194559-2c9e96a0b5d4.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
306-306: Verify compatibility withgolang.org/x/syncv0.8.0.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
303-303: Verify compatibility withgolang.org/x/modv0.20.0.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
18-18: Verify compatibility withcode.cloudfoundry.org/bytefmtv0.0.0-20240804182054-0a63f33a903d.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
173-173: Verify compatibility withgithub.com/ianlancetaylor/demanglev0.0.0-20240805132620-81f5be970eca.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
Verification successful
The module
github.com/ianlancetaylor/demangleis not currently used in the codebase. Therefore, updating its version tov0.0.0-20240805132620-81f5be970ecadoes not impact the existing code.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility with `github.com/ianlancetaylor/demangle` v0.0.0-20240805132620-81f5be970eca. # Test: Search for usage of `github.com/ianlancetaylor/demangle` in the codebase. rg --type go 'github.com/ianlancetaylor/demangle'Length of output: 49
Script:
#!/bin/bash # Description: Verify compatibility with `github.com/ianlancetaylor/demangle` v0.0.0-20240805132620-81f5be970eca. # Test 1: Search for import statements of `github.com/ianlancetaylor/demangle` in the codebase. rg --type go 'import "github.com/ianlancetaylor/demangle"' # Test 2: Search for any references to `demangle` functions or types in the codebase. rg --type go 'demangle'Length of output: 84
14-14: Verify compatibility withcloud.google.com/go/pubsubv1.41.0.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
Verification successful
No active usage of
cloud.google.com/go/pubsubfound in the codebase.The module is referenced in
go.modandgo.sum, but no usage was found in.gofiles. The version update to v1.41.0 should not impact the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility with `cloud.google.com/go/pubsub` v1.41.0. # Test: Search for usage of `cloud.google.com/go/pubsub` in the codebase. rg --type go 'cloud.google.com/go/pubsub'Length of output: 41
Script:
#!/bin/bash # Description: Verify compatibility with `cloud.google.com/go/pubsub` v1.41.0. # Test: Search for usage of `cloud.google.com/go/pubsub` in the codebase. rg 'cloud.google.com/go/pubsub' -g '*.go' -A 5Length of output: 46
Script:
#!/bin/bash # Description: Verify compatibility with `cloud.google.com/go/pubsub` v1.41.0. # Test: Search for usage of `cloud.google.com/go/pubsub` in the codebase, including go.mod and go.sum files. rg 'cloud.google.com/go/pubsub' -g '*.{go,mod,sum}' -A 5Length of output: 1319
53-53: Verify compatibility withgithub.com/aws/aws-sdk-go-v2/feature/s3/managerv1.17.10.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
305-305: Verify compatibility withgolang.org/x/oauth2v0.22.0.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
307-307: Verify compatibility withgolang.org/x/sysv0.23.0.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
Verification successful
No issues found with the update to
golang.org/x/sysv0.23.0.The changelog for version
v0.23.0indicates the addition of a newMountfunction for OpenBSD, with no breaking changes or modifications that affect existing functionality.
control_windows.go: No changes needed.control_unix.go: No changes needed.control_darwin.go: No changes needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility with `golang.org/x/sys` v0.23.0. # Test: Search for usage of `golang.org/x/sys` in the codebase. rg --type go 'golang.org/x/sys'Length of output: 237
Script:
#!/bin/bash # Description: Check the usage of `golang.org/x/sys` in the identified files. # Test: Search for usage of `golang.org/x/sys/windows` in control_windows.go rg 'golang.org/x/sys/windows' internal/net/control/control_windows.go -A 10 # Test: Search for usage of `golang.org/x/sys/unix` in control_unix.go rg 'golang.org/x/sys/unix' internal/net/control/control_unix.go -A 10 # Test: Search for usage of `golang.org/x/sys/unix` in control_darwin.go rg 'golang.org/x/sys/unix' internal/net/control/control_darwin.go -A 10Length of output: 940
Script:
#!/bin/bash # Description: Fetch and review the changelog for `golang.org/x/sys` version `v0.23.0`. # Fetch the changelog for golang.org/x/sys curl -s https://api.github.com/repos/golang/sys/releases | jq '.[] | select(.tag_name == "v0.23.0") | .body'Length of output: 110
Script:
#!/bin/bash # Description: Search for the changelog for `golang.org/x/sys` version `v0.23.0`. # Clone the golang/sys repository and search for the v0.23.0 changelog git clone https://github.com/golang/sys.git cd sys git log v0.23.0 -n 1Length of output: 753
96-96: Verify compatibility withgithub.com/envoyproxy/protoc-gen-validatev1.1.0.Ensure that the new version does not introduce any breaking changes or require additional modifications in the codebase.
pkg/gateway/lb/handler/grpc/handler_test.go (2)
5653-5653: LGTM! The renaming improves readability.The renaming of
wanttowantMergedandgottogotMergedenhances clarity.
5663-5665: Ensure consistency in renaming.Verify that all instances of
gotandwantare renamed togotMergedandwantMergedrespectively.charts/vald/values.yaml (3)
3251-3253: LGTM!The addition of the
kvs_background_sync_intervalparameter is well-documented and the default value of5sseems reasonable.
3254-3256: LGTM!The addition of the
kvs_background_compaction_intervalparameter is well-documented and the default value of5sseems reasonable.
3263-3265: LGTM!The addition of the
gatewayconfiguration is comprehensive and well-documented. The schema information is detailed and provides clear guidance on the usage of each parameter.charts/vald-helm-operator/crds/valdrelease.yaml (14)
7891-7897: Define addresses for the gateway.The
addrsproperty is correctly defined as an array of strings to specify gateway addresses.
7898-7914: Configure backoff settings.The
backoffobject includes properties such asbackoff_factor,backoff_time_limit,enable_error_log,initial_duration,jitter_limit,maximum_duration, andretry_count. These properties are crucial for managing retry and backoff strategies during failures.
7915-7917: Preserve unknown fields in call options.The
call_optionobject allows for additional configuration, preserving unknown fields. This is a good practice to ensure flexibility.
7918-7930: Manage circuit breaker behavior.The
circuit_breakerobject includes properties likeclosed_error_rate,closed_refresh_timeout,half_open_error_rate,min_samples, andopen_timeout. These properties are essential for managing the circuit breaker behavior.
7931-7943: Set connection pool parameters.The
connection_poolobject specifies parameters for managing connections, including DNS resolution and connection rebalancing.
7944-7966: Configure dial options.The
dial_optionobject includes properties that govern connection dialing, such asbackoff_base_delay,backoff_jitter,backoff_max_delay,backoff_multiplier,enable_backoff,initial_connection_window_size,initial_window_size,insecure,interceptors,keepalive,max_msg_size,min_connection_timeout,net,read_buffer_size,timeout, andwrite_buffer_size. These settings are comprehensive and cover various aspects of connection dialing.
7970-7977: Set keepalive properties.The
keepaliveobject withindial_optionincludes properties likepermit_without_stream,time, andtimeout. These settings ensure that connections are kept alive appropriately.
7982-8024: Configure network properties.The
netobject withindial_optionincludes properties fordialer,dns, andsocket_option. These settings are crucial for managing network-related configurations.
8025-8036: Enable TLS settings.The
tlsobject includes properties such asca,cert,enabled,insecure_skip_verify, andkey. These settings are essential for enabling and configuring TLS.
8043-8044: Set health check duration.The
health_check_durationproperty is defined as a string to specify the duration for health checks.
8045-8050: Configure message size limits.The properties
max_recv_msg_size,max_retry_rpc_buffer_size, andmax_send_msg_sizeare defined to enhance performance tuning by setting limits on message sizes.
8064-8065: Set wait for ready option.The
wait_for_readyproperty is defined as a boolean to ensure that the gateway waits for readiness before proceeding.
8084-8085: Add KVS background compaction interval.The
kvs_background_compaction_intervalproperty is defined as a string to specify the interval for background compaction.
8086-8087: Add KVS background sync interval.The
kvs_background_sync_intervalproperty is defined as a string to specify the interval for background synchronization.k8s/operator/helm/crds/valdrelease.yaml (10)
7891-7897: Ensureaddrsproperty is properly configured.The
addrsproperty should contain valid addresses for the gateway. Verify that the addresses are correctly specified and reachable.
7898-7914: Reviewbackoffconfiguration for correctness.The
backoffproperty includes several parameters likebackoff_factor,backoff_time_limit,enable_error_log,initial_duration,jitter_limit,maximum_duration, andretry_count. Ensure these parameters are set to appropriate values to handle retries and errors effectively.
7915-7917: Reviewcall_optionconfiguration.The
call_optionproperty is marked withx-kubernetes-preserve-unknown-fields: true. Ensure that this configuration is intentional and necessary for preserving unknown fields.
7918-7930: Reviewcircuit_breakersettings.The
circuit_breakerproperty includes parameters likeclosed_error_rate,closed_refresh_timeout,half_open_error_rate,min_samples, andopen_timeout. Ensure these parameters are configured to handle circuit breaking effectively.
7931-7943: Reviewconnection_poolsettings.The
connection_poolproperty includes parameters likeenable_dns_resolver,enable_rebalance,old_conn_close_duration,rebalance_duration, andsize. Verify that these settings are optimized for connection pooling and DNS resolution.
7944-7967: Reviewdial_optionsettings.The
dial_optionproperty includes several parameters likebackoff_base_delay,backoff_jitter,backoff_max_delay,backoff_multiplier,enable_backoff,initial_connection_window_size,initial_window_size,insecure,interceptors,keepalive,max_msg_size,min_connection_timeout,net,read_buffer_size,timeout, andwrite_buffer_size. Ensure these parameters are configured correctly for establishing and maintaining connections.
7978-8043: Reviewhealth_check_duration,max_recv_msg_size,max_retry_rpc_buffer_size,max_send_msg_size, andtlssettings.These properties enhance the configurability of health checks and message size limits. Ensure that the values for these properties are set to appropriate levels to maintain system health and security.
8044-8065: Ensurewait_for_readyis correctly configured.The
wait_for_readyproperty should be set to ensure that the system waits for the gateway to be ready before proceeding with operations.
8084-8085: Reviewkvs_background_compaction_interval.Ensure that the
kvs_background_compaction_intervalis set to an appropriate value to manage the compaction of the key-value store without impacting performance.
8086-8087: Reviewkvs_background_sync_interval.Ensure that the
kvs_background_sync_intervalis set to an appropriate value to manage the synchronization of the key-value store without impacting performance.
| // if test.afterFunc != nil { | ||
| // defer test.afterFunc(tt, test.args) | ||
| // } | ||
| // checkFunc := test.checkFunc | ||
| // if test.checkFunc == nil { | ||
| // checkFunc = defaultCheckFunc | ||
| // } | ||
| // | ||
| // got := WithBackgroundSyncInterval(test.args.dur) | ||
| // if err := checkFunc(test.want, got); err != nil { | ||
| // tt.Errorf("error = %v", err) | ||
| // } | ||
| // }) | ||
| // } | ||
| // } | ||
| // | ||
| // func TestWithBackgroundCompactionInterval(t *testing.T) { | ||
| // type args struct { | ||
| // dur time.Duration | ||
| // } | ||
| // type want struct { | ||
| // want Option | ||
| // } | ||
| // type test struct { | ||
| // name string | ||
| // args args | ||
| // want want | ||
| // checkFunc func(want, Option) error | ||
| // beforeFunc func(*testing.T, args) | ||
| // afterFunc func(*testing.T, args) | ||
| // } | ||
| // defaultCheckFunc := func(w want, got Option) error { | ||
| // if !reflect.DeepEqual(got, w.want) { | ||
| // return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", got, w.want) | ||
| // } | ||
| // return nil | ||
| // } | ||
| // tests := []test{ | ||
| // // TODO test cases | ||
| // /* | ||
| // { | ||
| // name: "test_case_1", | ||
| // args: args { | ||
| // dur:nil, | ||
| // }, | ||
| // want: want{}, | ||
| // checkFunc: defaultCheckFunc, | ||
| // beforeFunc: func(t *testing.T, args args) { | ||
| // t.Helper() | ||
| // }, | ||
| // afterFunc: func(t *testing.T, args args) { | ||
| // t.Helper() | ||
| // }, | ||
| // }, | ||
| // */ | ||
| // | ||
| // // TODO test cases | ||
| // /* | ||
| // func() test { | ||
| // return test { | ||
| // name: "test_case_2", | ||
| // args: args { | ||
| // dur:nil, | ||
| // }, | ||
| // want: want{}, | ||
| // checkFunc: defaultCheckFunc, | ||
| // beforeFunc: func(t *testing.T, args args) { | ||
| // t.Helper() | ||
| // }, | ||
| // afterFunc: func(t *testing.T, args args) { | ||
| // t.Helper() | ||
| // }, | ||
| // } | ||
| // }(), | ||
| // */ | ||
| // } | ||
| // | ||
| // for _, tc := range tests { | ||
| // test := tc | ||
| // t.Run(test.name, func(tt *testing.T) { | ||
| // tt.Parallel() | ||
| // defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) | ||
| // if test.beforeFunc != nil { | ||
| // test.beforeFunc(tt, test.args) | ||
| // } | ||
| // if test.afterFunc != nil { | ||
| // defer test.afterFunc(tt, test.args) | ||
| // } | ||
| // checkFunc := test.checkFunc | ||
| // if test.checkFunc == nil { | ||
| // checkFunc = defaultCheckFunc | ||
| // } | ||
| // | ||
| // got := WithBackgroundCompactionInterval(test.args.dur) | ||
| // if err := checkFunc(test.want, got); err != nil { | ||
| // tt.Errorf("error = %v", err) | ||
| // } | ||
| // }) | ||
| // } | ||
| // } |
There was a problem hiding this comment.
Re-implement the removed test functions.
The removal of TestWithBackgroundSyncInterval and TestWithBackgroundCompactionInterval alters the testing coverage for the pogreb package. Re-implementing these tests is crucial to ensure the reliability of background interval configurations.
| // func TestWithErrGroup(t *testing.T) { | ||
| // type args struct { | ||
| // eg errgroup.Group | ||
| // } | ||
| // type want struct { | ||
| // want Option | ||
| // } | ||
| // type test struct { | ||
| // name string | ||
| // args args | ||
| // want want | ||
| // checkFunc func(want, Option) error | ||
| // beforeFunc func(*testing.T, args) | ||
| // afterFunc func(*testing.T, args) | ||
| // } | ||
| // defaultCheckFunc := func(w want, got Option) error { | ||
| // if !reflect.DeepEqual(got, w.want) { | ||
| // return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", got, w.want) | ||
| // } | ||
| // return nil | ||
| // } | ||
| // tests := []test{ | ||
| // // TODO test cases | ||
| // /* | ||
| // { | ||
| // name: "test_case_1", | ||
| // args: args { | ||
| // eg:nil, | ||
| // }, | ||
| // want: want{}, | ||
| // checkFunc: defaultCheckFunc, | ||
| // beforeFunc: func(t *testing.T, args args) { | ||
| // t.Helper() | ||
| // }, | ||
| // afterFunc: func(t *testing.T, args args) { | ||
| // t.Helper() | ||
| // }, | ||
| // }, | ||
| // */ | ||
| // | ||
| // // TODO test cases | ||
| // /* | ||
| // func() test { | ||
| // return test { | ||
| // name: "test_case_2", | ||
| // args: args { | ||
| // eg:nil, | ||
| // }, | ||
| // want: want{}, | ||
| // checkFunc: defaultCheckFunc, | ||
| // beforeFunc: func(t *testing.T, args args) { | ||
| // t.Helper() | ||
| // }, | ||
| // afterFunc: func(t *testing.T, args args) { | ||
| // t.Helper() | ||
| // }, | ||
| // } | ||
| // }(), | ||
| // */ | ||
| // } | ||
| // | ||
| // for _, tc := range tests { | ||
| // test := tc | ||
| // t.Run(test.name, func(tt *testing.T) { | ||
| // tt.Parallel() | ||
| // defer goleak.VerifyNone(tt, goleak.IgnoreCurrent()) | ||
| // if test.beforeFunc != nil { | ||
| // test.beforeFunc(tt, test.args) | ||
| // } | ||
| // if test.afterFunc != nil { | ||
| // defer test.afterFunc(tt, test.args) | ||
| // } | ||
| // checkFunc := test.checkFunc | ||
| // if test.checkFunc == nil { | ||
| // checkFunc = defaultCheckFunc | ||
| // } | ||
| // | ||
| // got := WithErrGroup(test.args.eg) | ||
| // if err := checkFunc(test.want, got); err != nil { | ||
| // tt.Errorf("error = %v", err) | ||
| // } | ||
| // }) | ||
| // } | ||
| // } |
There was a problem hiding this comment.
Implement the test cases for TestWithErrGroup.
The function structure is well-defined, but the test cases need to be implemented.
Do you want me to implement the test cases or open a GitHub issue to track this task?
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }, | ||
| } | ||
| for _, daddr := range addrs { | ||
| if diff > 0 && daddr != addr { |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
282-349 lines are duplicate of pkg/index/job/correction/service/corrector.go:567-634 (dupl)
| for _, daddr := range addrs { | ||
| if diff > 0 && daddr != addr { | ||
| _, ok := found[daddr] | ||
| if !ok { |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
567-634 lines are duplicate of pkg/index/job/correction/service/corrector.go:282-349 (dupl)
| ) | ||
|
|
||
| // Option represents the functional option for index corrector. | ||
| type Option func(*correct) error | ||
|
|
||
| var defaultOpts = []Option{ | ||
| WithStreamListConcurrency(200), //nolint:gomnd | ||
| WithKvsAsyncWriteConcurrency(2048), //nolint:gomnd | ||
| WithStreamListConcurrency(200), //nolint:gomnd |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
directive //nolint:gomnd is unused for linter "gomnd" (nolintlint)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/v1.7 #2566 +/- ##
===============================================
Coverage ? 17.40%
===============================================
Files ? 566
Lines ? 69059
Branches ? 0
===============================================
Hits ? 12021
Misses ? 56241
Partials ? 797 ☔ View full report in Codecov by Sentry. |
kpango
deleted the
backport/release/v1.7/bugfix/index-correction/refactor-and-bugfix-index-correction
branch
fix: git add chart directory for release (#2356) (#2357) [patch] release v1.7.11 (#2358) :bookmark: :robot: Release v1.7.11 (#2360) Change docker scan timeout longer (#2363) (#2364) refactor code using golangci-lint (#2362) (#2365) Create SECURITY.md (#2367) (#2368) add commit hash build image (#2359) (#2371) update docker build target platform selection rules (#2370) (#2374) Make agent export index metrics to Pod k8s resource (#2319) (#2372) backport ci deps others (#2386) Update workflow to release readreplica chart (#2383) (#2387) :green_heart: :recycle: Add Con-Bench helm chart to the Vald charts (#2388) (#2389) Delete unnecessary code for mirror (#2366) (#2391) change JP logo to EN logo (#2369) (#2392) Add rotate-all option to rotator (#2305) (#2393) fix: build error of internal kvs test (#2396) (#2398) Resolve kvs already closed before last saving (#2390) (#2394) :robot: Update license headers / Format Go codes and YAML files (#2397) (#2400) create continous benchmark doc (#2352) (#2395) fix: disable protobuf dispatch for client (#2401) (#2403) update deps (#2404) (#2405) [patch] release v1.7.12 (#2406) :bookmark: :robot: Release v1.7.12 (#2408) :pencil: Fix typo of file name (#2413) (#2415) Fix agent-faiss build failed (#2418) (#2419) Add tests for index information export (#2412) (#2414) Fix the logic to determine docker image (#2410) (#2420) Update build rule for nightly image (#2421) (#2422) Fix output settings to determine-docker-image-tag action and release branch build tag name (#2423) (#2425) Add `index-operator` template implementation (#2375) (#2424) fix: typo of execution rule (#2426) (#2427) Backport Flush API (#2434) update deps & add validation for Flush API when agent is Read Only (#2433) (#2436) docs: add hrichiksite as a contributor for doc (#2441) (#2442) fix: bugfix version update for docker build (#2445) (#2446) Fix index job logic to pass DNS A record (#2438) (#2448) Added snapshot timestamp annotations to read replica agent (#2428) (#2443) Fix operator-sdk version (#2447) (#2449) add file name lint (#2417) (#2450) fix: add extra option for ci-container build (#2451) (#2452) Add base of benchmark operator dashboard (#2430) (#2453) Implement index operator logic for read replica rotation (#2444) (#2456) add inner product distance type for ngt (#2454) (#2458) Fix e2e for read replica and add e2e for index operator (#2455) (#2459) Add unit tests for index operator (#2460) (#2461) Bugfix recreate benchmark job when operator reboot (#2463) (#2464) Refactor k8s types (#2462) (#2465) :robot: Automatically update PULL_REQUEST_TEMPLATE and ISSUE_TEMPLATE (#2457) (#2469) Fix workflow trigger for backport pr creation (#2471) (#2472) Automatically add backport main label for release-pr (#2473) (#2475) update deps (#2468) (#2476) Implement client metrics interceptor for continuous benchmark job (#2477) (#2480) :chart_with_upwards_trend: Add client metrics panels for continuous benchmark job (#2481) (#2483) Update continuous benchmark docs (#2485) (#2486) Sync release/v1.7 to main (#2495) add read replica and rotator docs (#2497) (#2499) add reviewer guideline (#2507) (#2508) update large top-K ratio handling logic (#2509) (#2511) Change default image tag from latest to nightly (#2516) (#2518) Bugfix that caused an error when argument has 3 or more nil arguments (#2517) (#2520) add faiss in values.yaml & valdrelease.yaml (#2514) (#2519) capitalize faq (#2512) (#2522) Backport docs updates to release/v1.7 (#2521) [CI] Add workflow to synchronize ubuntu base image (#2526) (#2527) fix: update schedule (#2528) (#2530) refactor index manager service add index service API to expose index informations (#2525) (#2532) fix conflict bug (#2537) fix: make format (#2534) (#2540) Backport PR #2542, #2538 to release/v1.7 (#2543) fix: add checkout option (#2545) (#2546) Implement ngt Statistics API (#2539) (#2547) Add workflow to check git conflict for backport PR (#2548) (#2550) [create-pull-request] automated change (#2552) (#2556) Update dependencies, C++ standard, and improve Dockerfiles for better build systems and localization (#2549) (#2557) Backport #2559 (#2560) [BUGFIX] index correction process (#2565) (#2566) change external docker image reference to ghcr.io registry (#2567) (#2568) [patch] Release v1.7.13 (#2569) :bookmark: :robot: Release v1.7.13 (#2570) add HTTP2 support for http.Client and Vald HTTP Server (#2572) (#2575) Signed-off-by: kpango <[email protected]>
fix: git add chart directory for release (#2356) (#2357) [patch] release v1.7.11 (#2358) :bookmark: :robot: Release v1.7.11 (#2360) Change docker scan timeout longer (#2363) (#2364) refactor code using golangci-lint (#2362) (#2365) Create SECURITY.md (#2367) (#2368) add commit hash build image (#2359) (#2371) update docker build target platform selection rules (#2370) (#2374) Make agent export index metrics to Pod k8s resource (#2319) (#2372) backport ci deps others (#2386) Update workflow to release readreplica chart (#2383) (#2387) :green_heart: :recycle: Add Con-Bench helm chart to the Vald charts (#2388) (#2389) Delete unnecessary code for mirror (#2366) (#2391) change JP logo to EN logo (#2369) (#2392) Add rotate-all option to rotator (#2305) (#2393) fix: build error of internal kvs test (#2396) (#2398) Resolve kvs already closed before last saving (#2390) (#2394) :robot: Update license headers / Format Go codes and YAML files (#2397) (#2400) create continous benchmark doc (#2352) (#2395) fix: disable protobuf dispatch for client (#2401) (#2403) update deps (#2404) (#2405) [patch] release v1.7.12 (#2406) :bookmark: :robot: Release v1.7.12 (#2408) :pencil: Fix typo of file name (#2413) (#2415) Fix agent-faiss build failed (#2418) (#2419) Add tests for index information export (#2412) (#2414) Fix the logic to determine docker image (#2410) (#2420) Update build rule for nightly image (#2421) (#2422) Fix output settings to determine-docker-image-tag action and release branch build tag name (#2423) (#2425) Add `index-operator` template implementation (#2375) (#2424) fix: typo of execution rule (#2426) (#2427) Backport Flush API (#2434) update deps & add validation for Flush API when agent is Read Only (#2433) (#2436) docs: add hrichiksite as a contributor for doc (#2441) (#2442) fix: bugfix version update for docker build (#2445) (#2446) Fix index job logic to pass DNS A record (#2438) (#2448) Added snapshot timestamp annotations to read replica agent (#2428) (#2443) Fix operator-sdk version (#2447) (#2449) add file name lint (#2417) (#2450) fix: add extra option for ci-container build (#2451) (#2452) Add base of benchmark operator dashboard (#2430) (#2453) Implement index operator logic for read replica rotation (#2444) (#2456) add inner product distance type for ngt (#2454) (#2458) Fix e2e for read replica and add e2e for index operator (#2455) (#2459) Add unit tests for index operator (#2460) (#2461) Bugfix recreate benchmark job when operator reboot (#2463) (#2464) Refactor k8s types (#2462) (#2465) :robot: Automatically update PULL_REQUEST_TEMPLATE and ISSUE_TEMPLATE (#2457) (#2469) Fix workflow trigger for backport pr creation (#2471) (#2472) Automatically add backport main label for release-pr (#2473) (#2475) update deps (#2468) (#2476) Implement client metrics interceptor for continuous benchmark job (#2477) (#2480) :chart_with_upwards_trend: Add client metrics panels for continuous benchmark job (#2481) (#2483) Update continuous benchmark docs (#2485) (#2486) Sync release/v1.7 to main (#2495) add read replica and rotator docs (#2497) (#2499) add reviewer guideline (#2507) (#2508) update large top-K ratio handling logic (#2509) (#2511) Change default image tag from latest to nightly (#2516) (#2518) Bugfix that caused an error when argument has 3 or more nil arguments (#2517) (#2520) add faiss in values.yaml & valdrelease.yaml (#2514) (#2519) capitalize faq (#2512) (#2522) Backport docs updates to release/v1.7 (#2521) [CI] Add workflow to synchronize ubuntu base image (#2526) (#2527) fix: update schedule (#2528) (#2530) refactor index manager service add index service API to expose index informations (#2525) (#2532) fix conflict bug (#2537) fix: make format (#2534) (#2540) Backport PR #2542, #2538 to release/v1.7 (#2543) fix: add checkout option (#2545) (#2546) Implement ngt Statistics API (#2539) (#2547) Add workflow to check git conflict for backport PR (#2548) (#2550) [create-pull-request] automated change (#2552) (#2556) Update dependencies, C++ standard, and improve Dockerfiles for better build systems and localization (#2549) (#2557) Backport #2559 (#2560) [BUGFIX] index correction process (#2565) (#2566) change external docker image reference to ghcr.io registry (#2567) (#2568) [patch] Release v1.7.13 (#2569) :bookmark: :robot: Release v1.7.13 (#2570) add HTTP2 support for http.Client and Vald HTTP Server (#2572) (#2575) Signed-off-by: kpango <[email protected]>
fix: git add chart directory for release (#2356) (#2357) [patch] release v1.7.11 (#2358) :bookmark: :robot: Release v1.7.11 (#2360) Change docker scan timeout longer (#2363) (#2364) refactor code using golangci-lint (#2362) (#2365) Create SECURITY.md (#2367) (#2368) add commit hash build image (#2359) (#2371) update docker build target platform selection rules (#2370) (#2374) Make agent export index metrics to Pod k8s resource (#2319) (#2372) backport ci deps others (#2386) Update workflow to release readreplica chart (#2383) (#2387) :green_heart: :recycle: Add Con-Bench helm chart to the Vald charts (#2388) (#2389) Delete unnecessary code for mirror (#2366) (#2391) change JP logo to EN logo (#2369) (#2392) Add rotate-all option to rotator (#2305) (#2393) fix: build error of internal kvs test (#2396) (#2398) Resolve kvs already closed before last saving (#2390) (#2394) :robot: Update license headers / Format Go codes and YAML files (#2397) (#2400) create continous benchmark doc (#2352) (#2395) fix: disable protobuf dispatch for client (#2401) (#2403) update deps (#2404) (#2405) [patch] release v1.7.12 (#2406) :bookmark: :robot: Release v1.7.12 (#2408) :pencil: Fix typo of file name (#2413) (#2415) Fix agent-faiss build failed (#2418) (#2419) Add tests for index information export (#2412) (#2414) Fix the logic to determine docker image (#2410) (#2420) Update build rule for nightly image (#2421) (#2422) Fix output settings to determine-docker-image-tag action and release branch build tag name (#2423) (#2425) Add `index-operator` template implementation (#2375) (#2424) fix: typo of execution rule (#2426) (#2427) Backport Flush API (#2434) update deps & add validation for Flush API when agent is Read Only (#2433) (#2436) docs: add hrichiksite as a contributor for doc (#2441) (#2442) fix: bugfix version update for docker build (#2445) (#2446) Fix index job logic to pass DNS A record (#2438) (#2448) Added snapshot timestamp annotations to read replica agent (#2428) (#2443) Fix operator-sdk version (#2447) (#2449) add file name lint (#2417) (#2450) fix: add extra option for ci-container build (#2451) (#2452) Add base of benchmark operator dashboard (#2430) (#2453) Implement index operator logic for read replica rotation (#2444) (#2456) add inner product distance type for ngt (#2454) (#2458) Fix e2e for read replica and add e2e for index operator (#2455) (#2459) Add unit tests for index operator (#2460) (#2461) Bugfix recreate benchmark job when operator reboot (#2463) (#2464) Refactor k8s types (#2462) (#2465) :robot: Automatically update PULL_REQUEST_TEMPLATE and ISSUE_TEMPLATE (#2457) (#2469) Fix workflow trigger for backport pr creation (#2471) (#2472) Automatically add backport main label for release-pr (#2473) (#2475) update deps (#2468) (#2476) Implement client metrics interceptor for continuous benchmark job (#2477) (#2480) :chart_with_upwards_trend: Add client metrics panels for continuous benchmark job (#2481) (#2483) Update continuous benchmark docs (#2485) (#2486) Sync release/v1.7 to main (#2495) add read replica and rotator docs (#2497) (#2499) add reviewer guideline (#2507) (#2508) update large top-K ratio handling logic (#2509) (#2511) Change default image tag from latest to nightly (#2516) (#2518) Bugfix that caused an error when argument has 3 or more nil arguments (#2517) (#2520) add faiss in values.yaml & valdrelease.yaml (#2514) (#2519) capitalize faq (#2512) (#2522) Backport docs updates to release/v1.7 (#2521) [CI] Add workflow to synchronize ubuntu base image (#2526) (#2527) fix: update schedule (#2528) (#2530) refactor index manager service add index service API to expose index informations (#2525) (#2532) fix conflict bug (#2537) fix: make format (#2534) (#2540) Backport PR #2542, #2538 to release/v1.7 (#2543) fix: add checkout option (#2545) (#2546) Implement ngt Statistics API (#2539) (#2547) Add workflow to check git conflict for backport PR (#2548) (#2550) [create-pull-request] automated change (#2552) (#2556) Update dependencies, C++ standard, and improve Dockerfiles for better build systems and localization (#2549) (#2557) Backport #2559 (#2560) [BUGFIX] index correction process (#2565) (#2566) change external docker image reference to ghcr.io registry (#2567) (#2568) [patch] Release v1.7.13 (#2569) :bookmark: :robot: Release v1.7.13 (#2570) add HTTP2 support for http.Client and Vald HTTP Server (#2572) (#2575) Signed-off-by: kpango <[email protected]>
Description
The Index Correction logic has been fundamentally revised, bugfixed, and refactored, addressing the issue of missing indexes in the Replica=2 case. The internal Key-Value Store (KVS) has been transitioned from bbolt to pogreb. External dependent images are now uploaded to ghcr.io periodically via Cron Action to mitigate CI failures due to rate limits, as specified in the modified _docker_image.yaml after this PR merge.
Internal Configuration:
internal/config/corrector.go has been updated to include new fields: KVSBackgroundSyncInterval and KVSBackgroundCompactionInterval.
A new Gateway configuration has been added.
The corresponding test file corrector_test.go has been updated accordingly.
Helm Charts:
charts/vald-helm-operator/crds/valdrelease.yaml now includes new configuration options for the gateway and KVS intervals.
charts/vald/templates/index/job/correction/configmap.yaml has been modified to integrate the new configuration options.
Updates to charts/vald/values.schema.json and charts/vald/values.yaml support the new configuration settings.
Dockerfiles:
Minor adjustments in dockers/agent/core/agent/Dockerfile and dockers/dev/Dockerfile ensure consistency.
Kubernetes Configurations:
Several Kubernetes configuration files for index correction, creation, and save jobs, as well as the index operator, have been added and updated. These changes ensure proper integration and utilization of the new configuration options, addressing the index correction process's bug fix.
New Features:
Comprehensive configuration for the Vald indexing service, enhancing observability, server management, and connection handling.
New configuration options for gateway behavior, including properties for connection management and error handling.
New Kubernetes ConfigMaps for the Vald indexing operator and correction service streamline configuration management.
Background synchronization and compaction intervals support in the KVS configuration.
New functional options for the index corrector improve error handling and concurrency configuration.
New Dockerfiles for managing multi-stage builds.
Updated Kubernetes deployment configurations to enhance modularity and flexibility.
Bug Fixes:
Improved error handling for gateway client interactions and enhanced logic for managing vector corrections.
Documentation:
Updated configuration files and README to reflect new features and parameters for the Vald indexing service.
Refactor:
Simplified variable naming and structure across various components for better readability and maintainability.
Replaced hardcoded paths with dynamic variables in deployment scripts for improved flexibility.
Chores:
Updated dependencies to their latest versions for improved performance and security.
Updated version numbers for several components, indicating minor revisions and enhancements.
Related Issue
Versions
Checklist
Summary by CodeRabbit
New Features
vald-index-correction, automating scheduled operations.vald-index-creationandvald-index-save, specifying operational parameters.Bug Fixes
Documentation
Chores