fix: panic with concurrent schema parsing #502
Merged
Conversation
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
YousefHagag
force-pushed
the
fix_panic
branch
from
b066d96 to
8151732
Compare
YousefHagag
changed the title
YousefHagag
force-pushed
the
fix_panic
branch
from
448ee72 to
c1dc105
Compare
YousefHagag
force-pushed
the
fix_panic
branch
from
c3a2bd3 to
68aaa7c
Compare
nrwiersma
requested changes
Feb 10, 2025
ahmad-nabas
approved these changes
Feb 11, 2025
nrwiersma
requested changes
Feb 11, 2025
concurrent_parse_test.go
Outdated
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestConcurrentParse(t *testing.T) { |
Member
There was a problem hiding this comment.
Please add this to schema_test.go. The naming separates it from what it is trying to test.
Contributor
Author
There was a problem hiding this comment.
I have moved the test, also I have moved the schema to a file, it still reproduces the issue.
nrwiersma
approved these changes
Feb 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Issue: #481
This pull request introduces a new test for concurrent schema parsing, adds a method to merge schema caches, and updates the schema parsing logic to utilize an internal cache. The most important changes are as follows:
Testing Enhancements:
concurrent_parse_test.go: Added a new testTestConcurrentParseto validate concurrent schema parsing by running multiple goroutines that parse a test schema. This ensures the thread-safety and correctness of the parsing function.Schema Cache Enhancements:
schema.go: Added a new methodAddAllto theSchemaCachestruct, which allows merging all schemas from one cache into another. This is useful for combining schema caches without duplicating entries.Schema Parsing Logic Enhancements:
schema_parse.go: Updated theParseBytesWithCachefunction to use an internalSchemaCachefor intermediate parsing steps. This change helps in isolating the parsing process and then merging the results back into the original cache, improving the modularity and maintainability of the code.