fix: Synchronize EDR refresh #1633
Conversation
rafaelmag110
requested review from
paullatzelsperger and
wolf4ood
and removed request for
paullatzelsperger
rafaelmag110
force-pushed
the
synchronize_edr_refresh
branch
from
aecad9a to
944ff4d
Compare
core/edr-core/src/main/java/org/eclipse/tractusx/edc/edr/core/service/EdrServiceImpl.java
Outdated
Show resolved
Hide resolved
.../eclipse/tractusx/edc/edr/spi/testfixtures/index/lock/EndpointDataReferenceLockBaseTest.java
Outdated
Show resolved
Hide resolved
...re/src/main/java/org/eclipse/tractusx/edc/edr/core/lock/DefaultEdrLockProviderExtension.java
Outdated
Show resolved
Hide resolved
core/edr-core/src/main/java/org/eclipse/tractusx/edc/edr/core/lock/InMemoryEdrLock.java
Outdated
Show resolved
Hide resolved
core/edr-core/src/main/java/org/eclipse/tractusx/edc/edr/core/lock/InMemoryEdrLock.java
Outdated
Show resolved
Hide resolved
core/edr-core/src/test/java/org/eclipse/tractusx/edc/edr/core/lock/InMemoryEdrLockTest.java
Show resolved
Hide resolved
|
Actually, regarding the inmem lock; if access to the hashmap is atomic, the global lock isn't needed anymore. |
core/edr-core/src/main/java/org/eclipse/tractusx/edc/edr/core/lock/InMemoryEdrLock.java
Outdated
Show resolved
Hide resolved
Can't see how we can make it without row-level locks. Remember the row has to keep locked until the token is refreshed, that's how we keep its state consistent. I went for option two from your list. |
we could completely leave out locking altogether - this is the in-mem store, and it's not suited for production anyways. without row-level locks (i.e. only a global one), we'd effectively synchronize on the EDR store, which to me would be fine because - again this is in-mem-land. Option 2 is fine though. |
|
|
all ✅ |
* Failing test and added code to make it pass * Created module for EdrLockSql and added statements * alter test to check for correct response * remove debug messages * Introduce InMemoryEdrLock * working version * improve test for two different edrs. * code dup * refactor inmem acquireLock * handle * handle2 * refactor in mem EDR lock * update EdrServiceImpl to enable force refresh * Remove non existing job from verify.yaml * Add ComponentTests * fix failing uts * Removes global lock in inmem variant * inmem release lock should be atomic * retrigger CI * retrigger CI
* fix: Synchronize EDR refresh (#1633) * Failing test and added code to make it pass * Created module for EdrLockSql and added statements * alter test to check for correct response * remove debug messages * Introduce InMemoryEdrLock * working version * improve test for two different edrs. * code dup * refactor inmem acquireLock * handle * handle2 * refactor in mem EDR lock * update EdrServiceImpl to enable force refresh * Remove non existing job from verify.yaml * Add ComponentTests * fix failing uts * Removes global lock in inmem variant * inmem release lock should be atomic * retrigger CI * retrigger CI * updates due to edc 0.10.0 * Trigger CI * Trigger CI
WHAT
Introduces a synchronization mechanism with database locks that only allows one request to update an expired token at a time. All other requests wait until the first request updates the token and releases the database.
WHY
To fix the issue described in #1618
FURTHER NOTES
PR to bugfix/0.7.7 first because it's a priority to have that merged.
Closes #1618