Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable selective schema caching #151

Merged
merged 2 commits into from
Aug 12, 2022
Merged

Enable selective schema caching #151

merged 2 commits into from
Aug 12, 2022

Conversation

mostafa
Copy link
Owner

@mostafa mostafa commented Aug 10, 2022

Based on issue #146 reported by @oscar067 and the fix in PR #147 by the same awesome person, I created this PR to address the same issue from my own POV to achieve two objectives:

  1. Be able to enable/disable the caching of schema in the srclient package.
  2. Be able to choose which schemas get cached in this extension with the proposed caching solution by @oscar067.

@oscar067 WDYT?

@mostafa mostafa marked this pull request as ready for review August 10, 2022 22:08
@mostafa mostafa linked an issue Aug 10, 2022 that may be closed by this pull request
@sonarcloud
Copy link

sonarcloud bot commented Aug 10, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mostafa mostafa added the ✨ Feature Request New feature or request label Aug 11, 2022
@mostafa mostafa linked an issue Aug 11, 2022 that may be closed by this pull request
@mostafa mostafa merged commit 239f243 into main Aug 12, 2022
@mostafa mostafa deleted the selective-schema-caching branch August 12, 2022 17:25
@oscar067
Copy link

oscar067 commented Aug 13, 2022

Hi @mostafa ,

The way I use the xk6-kafka to test our microservices is by generating million of events which can be consumed by the microservice under test, that's because I need do the test with schema registry (consumers use it), and also I want to enable cache by default, because:

  • I do not want to test the perfomance of the schema registry.
  • I want xk6 to behave as other java/spark producers which also cache the schema.

I was testing the solution with master branch, as summary the test now produce 11414 and previously 257870 with 50vu in 1 minute.

I'm trying to figure out what's happening on master branch, any help will be appreciated :)

This is my schema registry configuration:


  schemaRegistry: {
    url: schemaRegistry,
    tls: {
      enableTls: enableTlsSchemaRegistry,
      insecureSkipTlsVerify: true,
      clientCertPem: "/certs/cert.pem",
      clientKeyPem:  "/certs/cert.key",
      serverCaPem:   "/certs/rootg3_b64.cer",
    },
    enableCaching: true
  },

And this is the test configuration:

export const options = {
  //I want it back :) discardResponseBodies: true,
  insecureSkipTLSVerify: true,
  teardownTimeout: '360s',
  tlsAuth: [
    {
      domains: [],
      cert: open('/certs/cert.pem'),
      key: open('/certs/cert.key')
    }
  ],
  vus: 50,
  duration: "1m"
}

Result is: 11414 message count
image

However with previous branch with the patch (https://github.com/oscar067/xk6-kafka) : 257870
image

@riferrei With previous branch I also did some tests using the sync.Map in https://github.com/oscar067/srclient/tree/v1.0.1 and it wrote 18614 (with previous xk6-kafka) master branch
image

Seems it improves the performance.

@mostafa
Copy link
Owner Author

mostafa commented Aug 13, 2022

@oscar067 The enableCaching: true parameter in the schemaRegistry configuration causes the caching of the srclient to be enabled, which I think isn't what you want. Instead, use the enableCaching: true parameter on the schema itself to enable the use of the xk6-kafka cache that you proposed.

Also, have you changed your code to use the changes in #149?

@oscar067
Copy link

oscar067 commented Aug 14, 2022

I didn't use the correct branch for doing the tests so I didn't apply the changes on my code.

Indeed what I want is configure on the schema itself :)

Having done the changes this is the result of the test, which clearly improves the previous version:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature Request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with Avro and schema registry Problem with schema registry cache- Workaround
2 participants