-
Notifications
You must be signed in to change notification settings - Fork 562
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
I can't activate jobs with a high max job count #5525
Comments
Let's make sure we properly handle not returning more jobs than we can fit in there. |
found an related old issue #1578 |
Linked error on camunda cloud. |
I'm downgrading this, as while I still think this is necessary for GA, it's not necessary immediately. |
Might be fixed or at least the chance that this happens is reduced by #5991 |
My current customer emphasized the importance of this issue again. |
@falko which version are they using? |
Last time my customer reproduced it 0.25.3. Now they are using 0.26 but trying to avoid the issue with workarounds |
Are you planning to include this in 1.0.0? |
It doesn't look like it at the moment - feel free to bring it up in the stakeholder meetings if you think it should have higher priority. |
8798: Add API to probe the logstream batch writer if more bytes can be written without writing them r=npepinpe a=npepinpe ## Description This PR adds a new API method, `LogStreamBatchWriter#canWriteAdditionalEvent(int)`. This allows users of the writer to probe if adding the given amount of bytes to the batch would cause it to become un-writable, without actually having to write anything to the batch, or even modify their DTO (e.g. the `TypedRecord<?>` in the engine). To avoid having dispatcher details leak into the implementation, an analogous method is added to the dispatcher, `Dispatcher#canClaimFragmentBatch(int, int)`, which will compare the given size, framed and aligned, with the max fragment length. This is the main building block to eventually solve #5525, and enable other use cases (e.g. multi-instance creation) which deal with large batches until we have a more permanent solution (e.g. chunking follow up batches). NOTE: the tests added in the dispatcher are not very good, but I couldn't come up with something else that wouldn't be too coupled to the implementation (i.e. essentially reusing `LogBufferAppender`). I would like some ideas/suggestions. NOTE: this PR comes out of the larger one, #8491. You can check that one out to see how the new API would be used, e.g. in the `JobBatchCollector`. As such, this is marked for backporting, since we'll backport the complete fix for #5525. ## Related issues related to #5525 Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
8798: Add API to probe the logstream batch writer if more bytes can be written without writing them r=npepinpe a=npepinpe ## Description This PR adds a new API method, `LogStreamBatchWriter#canWriteAdditionalEvent(int)`. This allows users of the writer to probe if adding the given amount of bytes to the batch would cause it to become un-writable, without actually having to write anything to the batch, or even modify their DTO (e.g. the `TypedRecord<?>` in the engine). To avoid having dispatcher details leak into the implementation, an analogous method is added to the dispatcher, `Dispatcher#canClaimFragmentBatch(int, int)`, which will compare the given size, framed and aligned, with the max fragment length. This is the main building block to eventually solve #5525, and enable other use cases (e.g. multi-instance creation) which deal with large batches until we have a more permanent solution (e.g. chunking follow up batches). NOTE: the tests added in the dispatcher are not very good, but I couldn't come up with something else that wouldn't be too coupled to the implementation (i.e. essentially reusing `LogBufferAppender`). I would like some ideas/suggestions. NOTE: this PR comes out of the larger one, #8491. You can check that one out to see how the new API would be used, e.g. in the `JobBatchCollector`. As such, this is marked for backporting, since we'll backport the complete fix for #5525. ## Related issues related to #5525 Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
8797: Extend Either/EitherAssert capabilities r=npepinpe a=npepinpe ## Description This PR extends `Either` by adding a new API, `Either#getOrElse(R)`. This allows to extract the right value of the `Either` or returning a fallback. I did not add any tests as the implementation is incredibly simple, and I can't foresee ever getting more complex, but do challenge this. It also extends the related `EitherAssert` by adding a new adding new `left` and `right` extraction capabilities. So you can now assert something like: ```java EitherAssert.assertThat(either).left().isEqualTo(1); EitherAssert.assertThat(instantEither) .right() .asInstanceOf(InstanceOfAssertFactories.INSTANT) .isBetween(today, tomorrow); ``` Note that calling `EitherAssert#right()` will, under the hood, still call `EitherAssert#isRight()`. This PR is related to #5525 and is extracted from the bigger spike in #8491. You can review how it's used there, specifically in the `JobBatchCollectorTest`. As such, this is marked for backporting, since we'll backport the complete fix for #5525. ## Related issues related to #5525 Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com> Co-authored-by: Nicolas Pepin-Perreault <43373+npepinpe@users.noreply.github.com>
8815: [Backport stable/1.2] Extend Either/EitherAssert capabilities r=npepinpe a=npepinpe ## Description This PR backports #8797 to stable/1.2, which is necessary to in the end backport the fix for #5525. There were some conflicts which I corrected with the last commit (one extra dependency pulled). ## Related issues backports #8797 Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com> Co-authored-by: Nicolas Pepin-Perreault <43373+npepinpe@users.noreply.github.com>
8814: [Backport stable/1.3] Extend Either/EitherAssert capabilities r=npepinpe a=npepinpe ## Description This PR backports #8797 to stable/1.3, which is necessary to in the end backport the fix for #5525. There were some conflicts which I corrected with the last commit (one extra dependency pulled). ## Related issues backports #8797 Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com> Co-authored-by: Nicolas Pepin-Perreault <43373+npepinpe@users.noreply.github.com>
Describe the bug
Reported by an user I have investigate an issue where it seems they are not able to activate more jobs. They changed the default configuration of the
maxMessageSize
to128KB
and experienced errors in the broker which indicate that job batch record is to large.To Reproduce
maxMessageSize
set to128KB
maxActivationCount
1000+If we set the FetchVariables to empty, then the broker seems to try to put more jobs into the job activation and fails on doing that.
Expected behavior
I can activate jobs without issues. If the max message size is reached it will still returned me jobs up to the specific limit.
Log/Stacktrace
https://console.cloud.google.com/logs/viewer?interval=NO_LIMIT&authuser=1&project=zeebe-io&minLogLevel=0&expandAll=false×tamp=2020-10-07T11:40:15.009000000Z&customFacets=&limitCustomFacetWidth=true&advancedFilter=resource.type%3D%22k8s_container%22%0Aresource.labels.project_id%3D%22zeebe-io%22%0Aresource.labels.location%3D%22europe-west1-b%22%0Aresource.labels.cluster_name%3D%22zeebe-cluster%22%0Aresource.labels.namespace_name%3D%22zell-small-msg-size%22%0Alabels.k8s-pod%2Fapp_kubernetes_io%2Fcomponent%3D%22broker%22%0Alabels.k8s-pod%2Fapp_kubernetes_io%2Finstance%3D%22zell-small-msg-size%22%0Alabels.k8s-pod%2Fapp_kubernetes_io%2Fmanaged-by%3D%22Helm%22%0Alabels.k8s-pod%2Fapp_kubernetes_io%2Fname%3D%22zeebe-cluster%22&scrollTimestamp=2020-10-07T11:39:38.166836000Z&pinnedLogId=hrguh6fg7uydd&pinnedLogTimestamp=2020-10-07T11:39:38.166836Z
Full Stacktrace
Environment:
The text was updated successfully, but these errors were encountered: