Skip to content

Conversation

@lpetrovic05
Copy link
Contributor

closes #20919

@lpetrovic05 lpetrovic05 added this to the v0.68 milestone Sep 25, 2025
@lpetrovic05 lpetrovic05 self-assigned this Sep 25, 2025
@lfdt-bot
Copy link

lfdt-bot commented Sep 25, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codacy-production
Copy link

codacy-production bot commented Sep 25, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 88939711 83.12%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (8893971) Report Missing Report Missing Report Missing
Head commit (1e7b322) 102926 77583 75.38%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#21235) 77 64 83.12%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@codecov
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

❌ Patch coverage is 57.50000% with 51 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...dera/node/app/quiescence/QuiescenceController.java 56.16% 22 Missing and 10 partials ⚠️
...ra/node/app/quiescence/QuiescenceBlockTracker.java 59.37% 8 Missing and 5 partials ⚠️
...om/hedera/node/app/quiescence/QuiescenceUtils.java 66.66% 2 Missing and 2 partials ⚠️
...dera/node/app/quiescence/BadMetadataException.java 0.00% 2 Missing ⚠️

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #21235      +/-   ##
============================================
- Coverage     71.82%   71.76%   -0.06%     
- Complexity    24371    24436      +65     
============================================
  Files          2647     2659      +12     
  Lines        103045   103365     +320     
  Branches      10780    10818      +38     
============================================
+ Hits          74007    74178     +171     
- Misses        25010    25147     +137     
- Partials       4028     4040      +12     
Files with missing lines Coverage Δ Complexity Δ
...m/hedera/node/app/quiescence/QuiescenceConfig.java 100.00% <100.00%> (ø) 1.00 <1.00> (?)
...dera/node/app/quiescence/BadMetadataException.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...om/hedera/node/app/quiescence/QuiescenceUtils.java 66.66% <66.66%> (ø) 7.00 <7.00> (?)
...ra/node/app/quiescence/QuiescenceBlockTracker.java 59.37% <59.37%> (ø) 8.00 <8.00> (?)
...dera/node/app/quiescence/QuiescenceController.java 56.16% <56.16%> (ø) 20.00 <20.00> (?)

... and 63 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
@lpetrovic05 lpetrovic05 force-pushed the 20919-quiescence-controller branch from 084086b to 00cd4fa Compare September 26, 2025 07:47
Signed-off-by: Lazar Petrovic <[email protected]>
s
Signed-off-by: Lazar Petrovic <[email protected]>
poulok
poulok previously approved these changes Sep 26, 2025
@lpetrovic05 lpetrovic05 marked this pull request as ready for review September 29, 2025 13:48
@lpetrovic05 lpetrovic05 requested review from a team as code owners September 29, 2025 13:48
Signed-off-by: Lazar Petrovic <[email protected]>
@tinker-michaelj
Copy link
Contributor

...

If this works, that would mean we don't need the controller. It's not clear to me that this covers everything, whether the cache keeps all the required data. Its definitely worth discussing.

Agreed, I don't think we need an explicit controller. The DeduplicationCache does keep all required data, since nodes populate it with every (user) TransactionID they have seen in pre-handle.

If nodes also remove user TransactionIDs as they are handled, then we want to quiesce exactly when the DeduplicationCache is empty.

@mhess-swl
Copy link
Contributor

mhess-swl commented Oct 1, 2025

If nodes also remove user TransactionIDs as they are handled, then we want to quiesce exactly when the DeduplicationCache is empty.

Two questions about this:

  1. How would we compute the TCT in this case?
  2. If a user submits a transaction to a quiesced network, how would the network break its quiescence? Seems the block stream manager would be the one to initiate quiescence, but a different component would have to break it

@derektriley
Copy link
Contributor

  • Stop adding HintsPartialSignature tx ids to the DeduplicationCache here (nodes will still be penalized for submitting duplicates; and they don't have to protect themselves from malicious users in choosing their ids for these txs.)
  • Add a remove(TransactionID) method to the DeduplicationCache and---iff quiescence is enabled---remove every user tx id after it is handled here (this could even be done off the handle thread, but performance impact should be minimal, especially on a network expecting low traffic in the first place).
  • Inject the singleton DeduplicationCache into BlockStreamManager; and, if the cache is empty at the end of a call to finishProofWithSignature(), then call quiesce().

Won't this open up the possibility of a duplicate transaction being submitted after it's been removed from the deduplication cache, and now will go through ingest, prehandle and be rejected during handle and charge fees vs being being rejected at ingest?

Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
Signed-off-by: Lazar Petrovic <[email protected]>
@codacy-production
Copy link

codacy-production bot commented Oct 8, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%) 71.67%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (279d90e) 102950 77991 75.76%
Head commit (ec946cd) 103159 (+209) 78151 (+160) 75.76% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#21235) 120 86 71.67%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

rbarker-dev
rbarker-dev previously approved these changes Oct 13, 2025
Signed-off-by: Lazar Petrovic <[email protected]>
@lpetrovic05 lpetrovic05 merged commit 58476d9 into main Oct 16, 2025
51 of 53 checks passed
@lpetrovic05 lpetrovic05 deleted the 20919-quiescence-controller branch October 16, 2025 14:23
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.

Create the quiescence controller

10 participants