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

POC: Zeebe db cache #12677

Closed
wants to merge 4 commits into from
Closed

POC: Zeebe db cache #12677

wants to merge 4 commits into from

Conversation

Zelldon
Copy link
Member

@Zelldon Zelldon commented May 5, 2023

Description

Idea was to cache key-value pairs during a transaction, so if requesting again some values we get them faster. This was straight forward to implement in the ZeebeTransaction itself.

Far more complicated would be a cache which exists over transactions bounds, since we need multiple layers of caches since transactions can be rolled back etc. The first iteration/commits goes into that direction but was far more complicated. Especially we need to make sure to make copies of the key-value pairs, otherwise we run into issues, since key and value instances are reused and changed during runtime. This can be overcome via a new API #12595.

JMH Benchmark doesn't show a significant impact, only in respect of the error rate.

EnginePerformanceTest.measureProcessExecutionTime  thrpt  200  225.579 ± 2.684  ops/s

Based on profiling we see that consistency check is the dominator of bad performance, which our cache can overcome, since we mostly request keys which doesn't exist yet (and in consequence are not in the cache).

Even with disabled consistency checks we can see that the gain is not much

EnginePerformanceTest.measureProcessExecutionTime  thrpt  200  442.611 ± 15.342  ops/s 

Again here we have a better error rate as without cache.

Related issues

closes #

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/1.3) to the PR, in case that fails you need to create backports manually.

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually
  • The change has been verified by a QA run
  • The impact of the changes is verified by a benchmark

Documentation:

  • The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
  • If the PR changes how BPMN processes are validated (e.g. support new BPMN element) then the Camunda modeling team should be informed to adjust the BPMN linting.

Other teams:
If the change impacts another team an issue has been created for this team, explaining what they need to do to support this change.

Please refer to our review guidelines.

Not that easy since we have several different key types, and in the ZeebeDB mostly they are generic, so we can align on a common sense which is the byte array or introduce some method to the DbKey interface, to identify the key like hashcode etc.

It would be easier if we refactor the API to allow direct types/objects see #12595 

In POC i tried it with using long hashcode

LRU Cache from agrona not that useful, used later the Cache impl (which is a map with capacity etc.)

Run again into the issue of reusing objects, especially values. The cache was corrupted by this, when storing just the values at somepoint all values have changed to the same value with different keys….

Caching is not that straightforward and lot of traps to fall into

Two layer caches, one for the current transaction and one for committed data?

Dont forget to delete data from cache…
only useful for inside transaction - i guess this is enough. Other solutions makes it too complicated and needs more caches (for each CF)
@Zelldon
Copy link
Member Author

Zelldon commented May 5, 2023

Closing for now, branch will stay to apply if we need it.

@Zelldon Zelldon closed this May 5, 2023
@Zelldon Zelldon deleted the zell-zeebe-db-cache branch March 28, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant