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

bugfix: avoid endless loops on contract negotiation sending failure #1489

Merged
merged 4 commits into from
Jun 21, 2022
Merged

bugfix: avoid endless loops on contract negotiation sending failure #1489

merged 4 commits into from
Jun 21, 2022

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Jun 16, 2022

What this PR changes/adds

Avoid endless loops on contract negotiation sending failures.

Why it does that

To avoid log flooding.

Further notes

  • used the SendRetryManager component, generalized to work also with negotiations and moved to the state-machine-lib module
  • introduced Entity interface, that could be a starting point for Core: base classes for entities and in-memory store #1463 (used an interface because it was the easiest thing to do, probably a class would fit better)
  • introduced an internal contract negotiation manager component called AsyncSendResultHandler that takes care of handling the async results, this was to remove a lot of duplication, and actually makes the code more readable.
  • I really think the SendRetryManager is pretty trivial to understand, I have in mind a refactor that could make everything more understandable, will propose it in another PR as this brought already enough modifications.

Linked Issue(s)

Closes #1487

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • added relevant details to the changelog? (skip with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

@codecov-commenter
Copy link

Codecov Report

Merging #1489 (2230ec4) into main (50c941a) will increase coverage by 0.00%.
The diff coverage is 80.95%.

@@           Coverage Diff           @@
##             main    #1489   +/-   ##
=======================================
  Coverage   67.16%   67.17%           
=======================================
  Files         716      716           
  Lines       15972    15989   +17     
  Branches     1057     1046   -11     
=======================================
+ Hits        10728    10740   +12     
- Misses       4770     4774    +4     
- Partials      474      475    +1     
Impacted Files Coverage Δ
...connector/transfer/core/CoreTransferExtension.java 0.00% <0.00%> (ø)
...sfer/core/transfer/TransferProcessManagerImpl.java 81.54% <ø> (ø)
...main/contract/negotiation/ContractNegotiation.java 0.00% <0.00%> (ø)
...tor/spi/types/domain/transfer/TransferProcess.java 76.47% <ø> (ø)
...iation/ConsumerContractNegotiationManagerImpl.java 86.89% <78.18%> (-0.89%) ⬇️
...iation/ProviderContractNegotiationManagerImpl.java 90.22% <78.57%> (-0.64%) ⬇️
...egotiation/AbstractContractNegotiationManager.java 94.04% <86.11%> (-5.96%) ⬇️
...mon/statemachine/retry/EntitySendRetryManager.java 89.47% <100.00%> (ø)
...ceconnector/contract/ContractServiceExtension.java 95.65% <100.00%> (+0.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50c941a...2230ec4. Read the comment docs.

@ndr-brt ndr-brt merged commit 9395de6 into eclipse-edc:main Jun 21, 2022
@ndr-brt ndr-brt deleted the bugfix/1487-avoid-endless-loop-contract-negotiation branch June 21, 2022 10:00
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.

core: if negotiation sending fails start loops endlessly
4 participants