Conversation
kellrott
reviewed
Jun 13, 2025
accounts/basic.go
Outdated
| if len(auth) > 0 { | ||
| user, password, ok := parseBasicAuth(auth[0]) | ||
| fmt.Printf("User: %s Password: %s OK: %s\n", user, password, ok) | ||
| log.Infof("User: %s Password: %s OK: %t\n", user, password, ok) |
Member
There was a problem hiding this comment.
We should probably start censoring the password
Collaborator
Author
There was a problem hiding this comment.
this has been since updated here https://github.com/bmeg/grip/blob/feature/indexing/accounts/basic.go#L32-L37
kellrott
reviewed
Jun 14, 2025
grids/optimize.go
Outdated
| return expanded | ||
| } | ||
|
|
||
| func GripOptimizer(pipe []*gripql.GraphStatement) []*gripql.GraphStatement { |
Member
There was a problem hiding this comment.
Maybe should be named GridsOptimizer?
Collaborator
Author
There was a problem hiding this comment.
this has also been updated here -- https://github.com/bmeg/grip/blob/feature/indexing/grids/optimizer.go#L13
Collaborator
Author
|
Cleaned up a lot of the logic. Added support for optimized nested field filtering and compound filters See updates in recent commit |
Member
|
Can you add an 'Experimental' warning to the logging for when the GRIDS driver is used? |
Dealing with module name change in benchtop
Fixes bugs with bulk delete function. Improves bulk delete function t…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR focuses on improving the performance of the GRIDS driver.
See benchtop PR bmeg/benchtop#6
OLD grids driver -- 3 months old. Some of the queries in the old driver didn't resolve correctly. See grip-benchmark repo for more thorough benchmark comparison
NEW driver using sonic, local caching, a different key structure. Note: some of the test cases have been expanded. This is not an apples to apples test, but gives a rough idea of speed improvements:
Adds indexing to Grids driver and SQL like drivers.
Fixes indexing in Mongo driver to conform to existing conformance tests
Refactors optimizer to be less annoying via a MATCH REPLACE pattern.
Implements a grids optimizer. Match matches a specific pipeline of statements and REPLACE specifies the custom grids executor to be used for that statement
Implements separate pebble index in grids so that V.has() and V().hasLabel() expressions can be more performant.
Works for nested fields using json path filter notation. See ot_index tests for details. Grids optimizer works on compound has statements.
Adds caching on stored data indices so that 2 benchtop lookup to figure out which table the data is in and where in the table the data is located can be reduced down to a cache get.
Removes asoc index for deletes and stores vertex label in value() for vertex key lookup
Tries, then scraps an idea of storing TO and FROM edge labels in the edge key in favor of a caching based solution since this would make it so that all edges in a series of bulk load commands would have to be loaded after all vertices since the way that ETL is done currently each bulk load command loads one vertex label type at a time it is not possible to know future vertex label types that have not been loaded yet when loading generated edges.
Reworks stored grids keys to be more like pebble keys. Removes "grid" storage style for an easier to use more
time performant but less space performant storage method.
Implements cache batch loading to speed this up loading saved indices from disk. This only applies on the first query after grip is restarted.
Also feature/kafka still needs a review.