Project

General

Profile

Actions
Copy link

Defect #2285

closed

Cache: null value is not recognised as cached value

Added by Radek Tomiška almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Radek Tomiška
Category:
Cache
Target version:
10.3.2
Start date:
05/29/2020
Due date:
% Done:

100%

Estimated time:
Affected versions:
Owner:

Description

Not cached or cached null value is not different:

return cache == null ? Optional.empty() : Optional.ofNullable(cache.get());

leads to Optional.empty() in both cases (if cached value - cache.get() - is null) => null values are not recognised as cached and value is loaded repetitivelly.

Cache manager interface has to be changed - Optional schould be removed => null value can be returned.

Actions
Copy link
 #1

Updated by Radek Tomiška almost 4 years ago

  • Target version set to 10.3.2
Actions
Copy link
 #2

Updated by Radek Tomiška almost 4 years ago

  • Status changed from New to In Progress
Actions
Copy link
 #3

Updated by Radek Tomiška almost 4 years ago

  • Subject changed from Cache: null values are not recognised as cached value to Cache: null value is not recognised as cached value
  • Description updated (diff)
Actions
Copy link
 #4

Updated by Radek Tomiška almost 4 years ago

  • % Done changed from 0 to 30
Actions
Copy link
 #5

Updated by Radek Tomiška almost 4 years ago

  • Status changed from In Progress to Needs feedback
  • Assignee changed from Radek Tomiška to Alena Peterová
  • % Done changed from 30 to 90

Three issues with cache were found and fixed:
- 'null' value was not recognised as cached value => api was changed (change added into changelog)
- cache size was too small by default (2000 entries only). I changed strategy and set Long.MAX_VALUE by default => cache can limit size if needed instead enlarge it by configuration. We used simple hash map without limit before distributed cache was implemented, so is more backward compatible too (change added into changelog).
- Cache initialization for synchronization executors was missing => no cache. I've added cache configuration.

Commit:
https://github.com/bcvsolutions/CzechIdMng/commit/2b081affbaf892b76f773b72e647b4db0363af97

Could you please provide me a feedback?

Actions
Copy link
 #6

Updated by Radek Tomiška almost 4 years ago

  • Description updated (diff)
Actions
Copy link
 #7

Updated by Alena Peterová almost 4 years ago

  • Assignee changed from Alena Peterová to Radek Tomiška
I tested the synchronization of organizations on our development environment (Windows + MS SQL) with 3800 organizations, it works now, thanks!
  • Every transformation for the "parent" attribute is called only once.
  • When comparing 10.1 vs. 10.3.2, it's now slightly slower (44 mins before upgrade, 52 mins now). (With 10.3.1, it was 10-15 times slower.)
  • Creation of an organization works.
  • Update of an organization (+ moving under a different parent) works. (Only the tree in GUI must be refreshed by Ctrl+F5 after the change, but that's probably nothing new.)

I tested also the provisioning brake, it works as expected.

I don't really feel competent to make code review, e.g. why are there 2 caches (acc:sync-mapping-cache, acc:sync-executor-cache). Maybe @sourek would be better candidate? :-)

Actions
Copy link
 #8

Updated by Radek Tomiška almost 4 years ago

  • Assignee changed from Radek Tomiška to Vít Švanda
Actions
Copy link
 #9

Updated by Vít Švanda almost 4 years ago

  • Status changed from Needs feedback to Resolved
  • Assignee changed from Vít Švanda to Radek Tomiška
  • % Done changed from 90 to 100

I did code review. Thanks for this fix.

Actions
Copy link
 #10

Updated by Radek Tomiška almost 4 years ago

  • Status changed from Resolved to Closed
Actions
Copy link

Also available in: Atom PDF