search

Azure / azure-cosmos-dotnet-v3

.NET SDK for Azure Cosmos DB for the core SQL API
MIT License
743 stars 495 forks source link

The information on the article is incorrect. You cannot set the AnalyticalStoreTimeToLiveInSeconds to null #4849

Open jsquire opened 1 month ago

jsquire commented 1 month ago

Issue Transfer

This issue has been transferred from the Azure SDK for .NET repository, #46814.

Please be aware that @hesperanca is the author of the original issue and include them for any questions or replies.

Details

Type of issue

Code doesn't work

Description

There has been a lot of confusion about how to disable the analytical store on a Cosmos container. I raised a bug about this some time ago that was dealt with by @ealsur. Some of the documentation has been amended, but this article still states that we can disable the CosmosDB integration by setting the TTL value to null, which is incorrect.

According to the updated documentation, to disable the analytical store we need to set the AnalyticalStoreTimeToLiveInSeconds property to 0 (not null as stated in this article).

It would also be great if it could be documented that setting the AnalyticalStoreTimeToLiveInSeconds property to 0 will not work if continuous backup is enabled (which is the default).

We are an ISV selling a SAAS application that is heavily dependent on CosmosDB. Documentation issues like this one can severely the service we provide to our customers.

Thanks for your help.

Page URL

https://learn.microsoft.com/en-us/dotnet/api/microsoft.azure.cosmos.containerproperties.analyticalstoretimetoliveinseconds?view=azure-dotnet

Content source URL

https://github.com/Azure/azure-docs-sdk-dotnet/blob/master/xml/Microsoft.Azure.Cosmos/ContainerProperties.xml

Document Version Independent Id

5d68eda7-bc03-d2b1-8f5b-e55207a4b839

Article author

@azure-sdk

Metadata

ealsur commented 1 month ago

@jsquire shouldn't this be an Issue on the Docs repository? The fact that:

It would also be great if it could be documented that setting the AnalyticalStoreTimeToLiveInSeconds property to 0 will not work if continuous backup is enabled (which is the default).

Is something that escapes the SDK, and should, ideally, be explained in the documentation (the correlation between the service features).

Ideally, this should be in: https://github.com/MicrosoftDocs/azure-databases-docs/

ealsur commented 1 month ago

@hesperanca for Docs specific feedback, use the Feedback button on the docs page:

Image

That normally creates an Issue on the repository for the documentation, tagging the author of the document.

hesperanca commented 1 month ago

@ealsur that's what I did but @jsquire closed that issue and move it here.

Same happened with a previous issue I've raised via documentation link. That one still opened since august.

Issues keep being moved around and no one takes responsibility to fix them 🙁

ealsur commented 1 month ago

@hesperanca It's a bit different I think. You might have created the Feedback on https://learn.microsoft.com/en-us/dotnet/api/microsoft.azure.cosmos.containerproperties.analyticalstoretimetoliveinseconds?view=azure-dotnet

This is the SDK API, and that gets routed to the SDK repositories. The Issue template shows that that's where it was created.

This is not an SDK scenario though, 0 is the right value to disable. But the fact that another service feature (continuous backup it seems) is blocking that operation, is something the SDKs cannot know.

I'm sorry that you had a bad experience, do you have a link to the issue created on the Documentation page (not the SDK API)? Maybe we can get some attention there.

jsquire commented 1 month ago

@jsquire shouldn't this be an Issue on the Docs repository? The fact that:

It would also be great if it could be documented that setting the AnalyticalStoreTimeToLiveInSeconds property to 0 will not work if continuous backup is enabled (which is the default).

Is something that escapes the SDK, and should, ideally, be explained in the documentation (the correlation between the service features).

Ideally, this should be in: https://github.com/MicrosoftDocs/azure-databases-docs/

I don't have enough context to offer an opinion on where it should live. I can only assess that it is not something that would see attention or help if left in the Azure SDK for .NET repository where it was routed. If you believe it would have a better chance of being addressed in the main docs repository, please feel free to transfer it there.

hesperanca commented 1 month ago

@ealsur sorry, normally raise the issue in the page where I find it.

The other issue still open since August is this one: https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4635

Also, just to add some context here, we recently had a priority 1 problem in our production environment that severely affected us and I think it was related to this. The issue has now been fixed and I’m waiting for clarification on the RCA report (TrackingID#2410130050001040).

Many thanks.

ealsur commented 3 weeks ago

@hesperanca The other issue you are linking is unrelated to what is discussed in this thread.

This thread is about the interaction of two service features (continuous backup and analytical store). There is nothing that can be changed on the client SDK side that can remediate what you are seeing. I do agree that the documentation should be updated to reflect the caveats of these 2 features and the proper way would be to send a Feedback on the article itself: https://learn.microsoft.com/en-us/dotnet/api/microsoft.azure.cosmos.containerproperties.analyticalstoretimetoliveinseconds?view=azure-dotnet

hesperanca commented 3 weeks ago

@ealsur Thanks for your reply.

I found a documentation issue so I've followed the link at the bottom of the article that says "Open a documentation issue". Which looked like the logical thing to do!!!

Image

That automatically takes me to the "azure-sdk-for-net" GitHub repo where the original issue was raised. That issue was then closed by your colleague @jsquire and moved to the "azure-cosmos-dotnet-v3" repo.

I understand what you are saying about not being an issue with the client SDK but that doesn't change the fact that the documentation is incorrect. Who should fix the SDK documentation I do not know but from my perspective I followed what, to me, looked like the proper path to raise the documentation issue.

Just to clarify here are the issues with that article:

I hope the above make sense.

ealsur commented 3 weeks ago

I understand how confusing it might be, it would certainly throw me off too.

I think the UI is generic in the sense that the "Open a documentation issue" applies to the Method you are looking. This makes sense for the cases where the code is wrong (as your other Issue explains), that is a valid SDK Issue.

Let's split the cases:

  1. The screenshots you are showing are for the SDK property. We fixed that and the source code no longer contains that information.

https://github.com/Azure/azure-cosmos-dotnet-v3/blob/018dd2073b4f2697069de2b3e73af7cdae876f70/Microsoft.Azure.Cosmos/src/Resource/Settings/ContainerProperties.cs#L609

This was the PR that updated that: https://github.com/Azure/azure-cosmos-dotnet-v3/pull/4638

These docs however are auto-generated, it seems that they have not been updated with recent releases. @kirankumarkolli it seems the SDK API docs are missing updating to the latest source code.

  1. Going to the original topic of this issue: That you cannot disable analytical store if continuous backup is enabled. This is a product level feedback (this is not something enforced by client SDKs) and as such, the best possible path would be to report or provide feedback on the product page: https://learn.microsoft.com/en-us/azure/cosmos-db/analytical-store-introduction#analytical-ttl, that way, the explanation and any restrictions, can be added there, because they apply regardless of the SDK language/version or even if you do this by REST API. Would that be possible? That way the feature owners can address that as part of the service documentation. If you have already done this and received no response, please share the Issue that was created for it.
hesperanca commented 3 weeks ago

Thanks for the quick reply and for your help with this.

I will raise the other issue in the product page as per your suggestion.

Once again. Thanks for your help.