-
Notifications
You must be signed in to change notification settings - Fork 507
NotImplementedException in change feed handler #4093
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
Comments
This is by design. Estimator response is not coming from a Network request (it is a local calculation) so there are no Network Headers. |
@mortenbock The stack trace points to OpenTelemetry, are you using 3.35.3 or 3.35.3-preview? |
@sourabh1007 Is OpenTelemetry in this case always accessing the Headers? This will never work on the Estimator, the responses are not Network responses. |
@ealsur Apologies, you are right, the application is using (There were two applications writing to the same logs) |
@mortenbock If you are not using the OpenTelemetry integration, you can disable it with CosmosClientOptions.IsDistributedTracingEnabled = false. That should remove the exception, while we investigate. |
@ealsur We were using it, but I'm migrating away from the -preview packages now to disable it, because it was incredibly chatty, but that is a discussion for a separate issue :) |
@mortenbock Please provide feedback about OpenTelemetry integration in a separate Issue, it's very valuable to us. |
@mortenbock Looking at this more in detail: azure-cosmos-dotnet-v3/Microsoft.Azure.Cosmos/src/ChangeFeedProcessor/ChangeFeedEstimatorIterator.cs Lines 169 to 174 in 4556567
This only happens if there is no Change Feed Processor running, is this expected in your case? Estimator is meant to be used with a running Processor, unless you have a misconfiguration? |
@ealsur I think in our case, the code for the Change Feed Processor was active, but the container that it was listening for changes from was empty/new, so there were no documents in there. I don't know if that covers the same case? |
@mortenbock No, the condition for this to happen is the Lease Container is empty. This means, a Processor was never running/never started on that Lease Container. Leases are created when the Processor starts for the first time. The above Which means either there is a misconfiguration (your estimator is not pointing to the lease container used by your Processor) or there is a fundamental failure (you are using the Estimator without a Processor to monitor). This does not remove the need to solve the exception in Open Telemetry, but it is pointing to something not being correct in the way you are using the Estimator. |
@ealsur Thanks you for the feedback. Looking at our code, we are starting the estimator before the processor like this:
So I guess I could swap the order of those two, so the estimator does not start before the processor has created a lease document the first time the application deploys? If we were to deploy the estimator separately, would there be a good way to check if a lease exists before starting the estimator? |
There's no correlation between when you start the processor or estimator and when the lease/checkpoint document/item is written - i.e., one doesn't wait for the other, and there is no guaranteed max time that one could sleep for. Question: If the estimator is left running, will it work once a lease has been established, or does the absence of one prevent it from starting its normal cycle? ... And a bump for the programmatic way to trigger deletion or rewinding of the checkpoint request: #510 |
Ideally you want the Estimator in a separate instance. Reference: https://learn.microsoft.com/en-us/azure/cosmos-db/nosql/how-to-use-change-feed-estimator?tabs=dotnet#estimator-deployment
You should not monitor something from within itself. If the Processor machine goes into a bad state for whatever reason, there goes your monitoring too.
Yes, it will eventually pick-up the leases in one of their estimations. |
Using the 3.35.3 nuget package, I'm getting this in my logs;
Thrown from here;
azure-cosmos-dotnet-v3/Microsoft.Azure.Cosmos/src/ChangeFeedProcessor/ChangeFeedEstimatorIterator.cs
Line 405 in 258d960
The text was updated successfully, but these errors were encountered: