Repository navigation
Pause usage sharing sends after repeated failures - #62
Merged
Merged
Conversation
Usage sharing in Java has no back-off. Every batch is sent while the service is failing, and is lost. These tests expect the back-off .NET is being given, at its default settings of ten failed sends within 100 seconds and a first pause of two seconds: - after ten failed sends nothing is sent during the pause, and the evidence processed during it is all sent once the pause ends and the service answers, with no more requests processed; - closing while paused returns at once and sends nothing. They fail until the back-off is added.
Java now has the same back-off as .NET, with the same five settings and defaults. - After setFailuresToEnterRecovery sends fail within setFailuresWindowSeconds, sending pauses for setInitialRecoveryDelaySeconds. A send fails when it throws or the answer is not 200. - When a pause ends one send is tried. If it fails the delay is multiplied by setRecoveryMultiplier, up to setMaxRecoveryDelaySeconds. If it succeeds the back-off ends. - Processing is not affected. Evidence is queued during a pause and new evidence is dropped only when the queue is full. - The consumer waits out the pause itself, on a latch that closing releases, and then sends until fewer than the minimum entries per message are queued, so the queue drains when a pause ends without more requests being processed. - Closing ends a pause at once and sends nothing while paused, logging how many entries were not sent. ShareUsageBackOff holds the rules and takes the time from the caller, so its tests set the time. The consumer calls the new protected waitWhilePaused before each message and recordSendResult after it. New constructors take the five settings, and the existing ones use the defaults.
The builder setter rejects a value outside 1 to 100, and the back-off now does too, so a class derived from ShareUsageBase gets the same check through its constructor.
The comments in the functions this change touches described what the code used to do. They now say only what the code does and why.
This was referenced Nov 7, 2026
…ing parameters, read keys without regard to case, and keep one tracker cache
jwrosewell
marked this pull request as ready for review
November 11, 2026 15:22
This was referenced Nov 11, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Java's usage sharing has no back-off, so every message is sent, and lost, while the service is failing. This adds the back-off that 51degrees-dotnet#88 and 51degrees-go#84 already have, with the same five settings and defaults.
Problem
ShareUsageElement.sendUsageDatasent every batch it took from the queue, and a failed send was logged and the batch dropped. During an outage every batch is lost, and each one is another request to a failing service. .NET had a back-off from pipeline-dotnet#218, but it stopped processing as well as sending and was removed in pipeline-dotnet#261.What the specification says
The Back-off after failed sends section of usage-sharing-element.md, pinned at 8aa2de01 on specifications
rc, came in with specifications#97, which has merged. Its six rules and where each one lives in Java:ShareUsageBackOff.failedShareUsageBackOff.failedShareUsageBackOff.succeededShareUsageBase.processInternalShareUsageElement.sendUsageDatawithShareUsageBase.waitWhilePausedShareUsageBase.recordSendResultThe section also says a send fails when the request cannot be made, has no answer within the time the implementation allows, or is answered with anything other than 200, and that the batch that failed is dropped rather than put back on the queue.
What changed
setFailuresToEnterRecoverysetFailuresWindowSecondssetInitialRecoveryDelaySecondssetMaxRecoveryDelaySecondssetRecoveryMultiplierShareUsageBackOffholds the rules. Each method takes the time from the caller, onSystem.nanoTimethroughShareUsageBackOff.now, so a change to the system clock cannot lengthen or skip a pause, and the tests set the time.ShareUsageBasegains the protectedwaitWhilePausedandrecordSendResult, whichsendUsageDatacalls before and after each message. A class derived fromShareUsageBasewith its own consumer gets no back-off until it calls them.CountDownLatchthat closing releases, so closing ends a pause at once and sends nothing more, and it logs how many entries were not sent.ShareUsageBaseandShareUsageElementtake the five settings, and the existing ones pass the defaults, so code built against them is unchanged. The defaults are inConstantsunder the .NET names.sendAsXMLreports the status and the content of a failed answer, which the specification requires to aid troubleshooting, andDataUploaderHttpgainsgetResponseContentand a read timeout so a send that is never answered ends rather than stopping all sending.QUEUE_FULL_LOG_EVERYentries, never one message per entry, which is what the Error handling section forbids.The branch also carries the usage sharing options to share all Evidence and all query string parameters, to read keys without regard to case, and to keep one tracker cache.
Tests
The tests were written first, in
ShareUsageElementBackOffTests.java,ShareUsageBackOffTests.java,ShareUsageOptionsTests.java,ShareUsageSendTimeoutTests.javaandDataUploaderHttpTests.java.rcwith only the two tests that compile there: run 42347851766.ShareUsageElement_BackOff_PausesThenDrainsQueuesaw thirteen sends 0.7 seconds after the tenth failed send where ten were expected, andShareUsageElement_BackOff_CloseWhilePausedDoesNotWaitOrSendsaw a send during the pause. The other 27 usage sharing tests passed, with one skipped as before.The two tests that set the delays need the new constructor, so they were added with the change, as were
ShareUsageBackOffTestsand the setter checks.Two findings fixed since those runs
The empty deque
ShareUsageBackOff.failedtestedfailures.peekFirst()in the loop condition, which unboxes aLongand would throw aNullPointerExceptionon an empty deque. The deque is now tested for emptiness first, and the comment says the constructor refuses a window below one second, so the case cannot be reached. There is no red run for it, because nothing the constructor allows reaches the branch and a test that reached it would have to weaken the constructor first. 51degrees-python#53 has the same guard for the same reason.A new test broke an existing one, and the mechanism is worth knowing
The first three builds of this branch failed on
HttpClientTests.VerifyHeaders, with the cloud answering 401 "This resource key is not authorised for the domain ''". The same test passed in another draft's build on the samercat 12:46, 12:49 and 12:51, spanning the first failure, so the service and the key were working and this branch was the difference.HttpClientDefault.connectsetssun.net.http.allowRestrictedHeadersto true and then opens the connection.Originis on the JDK's restricted header list, andsun.net.www.protocol.http.HttpURLConnectionreads that property once, in its static initialiser, into astatic finalfield. Whichever code first loads that class therefore decides for the whole run whetherOriginis sent.The new
DataUploaderHttpTestsopens anHttpURLConnectionand did not set the property. It sorts beforeHttpClientTestsand the log shows it running sixteen seconds earlier in the same JVM, so it loaded the class first, the property was unset,Originwas dropped from then on, and the cloud saw an empty domain. The test now sets the same property in a@BeforeClass, so the class is loaded with the valueHttpClientDefaultneeds whichever test runs first.The three failures, for the record: 43020842228 twice and 43031127409. In the build below
HttpClientTestspasses two of two in every one of the nine jobs.Full build
Run 43033176993, success, on head
17f66db227773271f461d7e2a45e265d98751208. Configure, the nine build and test jobs (Ubuntu x64 with Java 8, 11, 17 and 21, Ubuntu ARM with Java 17, and Windows with Java 8, 11, 17 and 21) and Complete all passed. All 10Setting 'PullRequestSha'lines are17f66db227773271f461d7e2a45e265d98751208, the head, and the Configure log reads "Left out 6 macOS entries".macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.
The branch takes
rcat ce406b2, with no conflict.What remains
Neither is a defect in this change, and both want an issue of their own.
Originfragility is older than this change.DataUploaderHttpalready opened connections before this draft, so an application that shares usage before anything callsHttpClientDefault.connectis already losingOrigin. Fixing it properly meansHttpClientDefaultnot depending on which class loads first. This change only stops the tests tripping it.ShareUsageBuilderBaseTestuses JUnit 4 annotations and did not run in any of the runs above, which ran every other usage sharing test class. Whether the build runs JUnit 4 tests at all has not been checked.For review
jdkhttp.java, a file copied into the clone by mistake while working out theOriginfailure. The tree is clean and the diff againstrcnever held it, and tidying the history would have needed a force push.The same back-off in the other languages
Related
Still to do