[BACKLOG-39705] Remove from Region Cache not removing 1st level cache…

[tkafalas]

… entry.

[NJtwentyone]

I'm normally don't care for updating commit message but I feel in this instance, it make sense to add [PPP-4826] since there isn't really any direct code change to anything Manage datasource in PUC. Also, it might prevent any issue with backporting this change to previous releases.

[tkafalas]

probably adding [ppp-4826] is a good idea. I leave the proof to the QA team. I have seen it locally. Please feel free to run it thought the IT test since you are in a good position to do that.

[NJtwentyone]

@tkafalas Do you provide any proof that the code changes works? Either the steps to reproduce noted in https://hv-eng.atlassian.net/browse/BACKLOG-39705 And some execution of integration tests mentioned here: https://hv-eng.atlassian.net/browse/BACKLOG-39705?focusedCommentId=1940152

For my POC fix (https://hv-eng.atlassian.net/browse/BACKLOG-39705?focusedCommentId=1940968) I had to update a couple more functions for get the integration test to pass like CacheManager#clearRegionCache( String region )

[NJtwentyone]

Not a blocker

For CacheManager#removeFromRegionCache shouldn't be a catch statement statement here and at least a log line here, instead of shallowing the error.
The log level can be [error, debug, trace] just people can debug it in the field without a code change.

[tkafalas]

The try is to guarantee that open session gets closed. an exception will bubble up

[NJtwentyone]

Not a blocker.

similar comment for https://github.com/pentaho/pentaho-platform/pull/5511/files#r1471722021

shouldn't we add a log statement with level [error, debug, trace,..] for MappingException to class that will handle hvcache.evictEntityData( (String) key );

pentaho-platform/extensions/src/main/java/org/pentaho/platform/plugin/services/cache/LastModifiedCache.java

Lines 135 to 141 in cefd702

	@Override public void evictEntityData( String entityName ) {
	try {
	evictEntityData( getSessionFactory().getMetamodel().entityPersister( entityName ) );
	} catch ( MappingException e) {
	//Nothing to do if the entry is not there.
	}
	}

The MappingException was being shallowed and there was no way to find this out except for a developer attaching a debugger.

[tkafalas]

I don't believe so. That exception happens when trying to remove the 2nd level cache which is only used in the carteStatusCache to persist the log xml to a file. The fact that there is no 2nd level cache persister for these entries is not an error and logging it would just produce spam.

[NJtwentyone]

The use case would be if we need to customer's environment which we don't have access, but they can give us log files. So would don't have to deploy custom jars and we can't attach debuggers. Customer support or whoever would simply have to adjust logging levels in log4j.

if you find it useful or not address it or not. I'm fine either way.

[hitachivantarasonarqube]

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

[buildguy]

Note:

---

Frogbot also supports Contextual Analysis, Secret Detection, IaC and SAST Vulnerabilities Scanning. This features are included as part of the JFrog Advanced Security package, which isn't enabled on your system.

---

🐸 JFrog Frogbot

[buildguy]

⚠️ Build finished in 1h 30m 51s

Build command:

mvn clean verify -B -e -Daudit -amd -pl extensions

---

⛔ Failed Tests

⛈️ 1 test(s) failed:

org.pentaho.platform.plugin.services.exporter.PentahoPlatformExporterTest.testExportSchedules_SchedulereThrowsException (click to expand)

Wanted but not invoked:
iScheduler.getJobs(null);
-> at org.pentaho.platform.plugin.services.exporter.PentahoPlatformExporterTest.testExportSchedules_SchedulereThrowsException(PentahoPlatformExporterTest.java:154)
Actually, there were zero interactions with this mock.

Tests run: 1544, Failures: 1, Skipped: 6    Test Results

---

ℹ️ This is an automatic message