search

Azure / azure-sdk-for-go

This repository is for active development of the Azure SDK for Go. For consumers of the SDK we recommend visiting our public developer docs at:
https://docs.microsoft.com/azure/developer/go/
MIT License
1.64k stars 844 forks source link

Table Storage - Batch Insert only inserts one record when using Azurite #14750

Closed davidandradeduarte closed 2 years ago

davidandradeduarte commented 3 years ago

Bug Report

What happened?

Trying to insert multiple records into a table storage using batch mode doesn't work as expected and only inserts one. This only happens when using Azurite.

What did you expect or want to happen?

When running t.ExecuteBatch I'd expect it to insert all the entities added to the batch using t.InsertEntity. I've tried with a different SDK (.NET) and it works.

How can we reproduce it?

https://github.com/davidandradeduarte/azurite-insert-batch-bugs#batch-insert-with-go-sdk-using-azurite-only-inserts-one-record This repository reproduces the issue. You just need to have Azurite running on default ports. You can use the dotnet-sdk version included to test alongside and see that it works as expected.

ArcturusZhang commented 3 years ago

Hi @davidandradeduarte thanks for this issue!

Hi @jhendrixMSFT could you please take a look?

davidandradeduarte commented 3 years ago

hi 👋 @ArcturusZhang, @jhendrixMSFT any update on this?

jhendrixMSFT commented 3 years ago

Sorry for the delay.

The tables functionality in the storage package has been deprecated in favor of our new aztables module. Can you please see if this resolves your issue?

davidandradeduarte commented 2 years ago

Thanks, I'll have a look and let you know!

davidandradeduarte commented 2 years ago

Having a hard time trying to make it work.

Following this guide: https://devblogs.microsoft.com/azure-sdk/announcing-the-preview-release-for-azure-tables-for-go/

I've updated the code as follows: https://github.com/davidandradeduarte/azurite-insert-batch-bugs/blob/main/go-sdk/14750-aztables-fix/main.go

But fails with a 400 - Bad Request when I call client.Create (https://github.com/davidandradeduarte/azurite-insert-batch-bugs/blob/9767b15c5c5095df8a82af439d9cbab2102fc656/go-sdk/14750-aztables-fix/main.go#L21).

Azurite response 172.17.0.1 - - [26/Nov/2021:13:26:43 +0000] "POST /Tables HTTP/1.1" 400 -

Any idea? I don't see any good docs for aztables + Azurite. Only for the old sdk.

davidandradeduarte commented 2 years ago

Following up,

Using the aztables.NewServiceClientFromConnectionString seems to go further.

Now I'm getting: "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\n<Error>\n <Code>InvalidOperation</Code>\n <Message>Invalid storage account.\nRequestId:0f03ae6e-6929-40b4-8c0d-6e414a137857\nTime:2021-11-26T14:28:06.603Z</Message>\n</Error>\nunmarshalling type *internal.TableServiceError: invalid character '<' looking for beginning of value

The code is available at: https://github.com/davidandradeduarte/azurite-insert-batch-bugs/blob/main/go-sdk/14750-aztables-fix/main_conn.go#L23

Edit: same code works with a real Azure storage account

seankane-msft commented 2 years ago

@davidandradeduarte thanks for reporting this issue. I was able to get your latest snippet working by changing the connection string slightly:

Currently you have: DefaultEndpointsProtocol=http;AccountName=dummyaccountname;AccountKey=secretkeykey;TableEndpoint=http://localhost:10002/TestTable;.

I used DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;TableEndpoint=http://127.0.0.1:10002/devstoreaccount1; and was able to successfully run the snippet. Can you try this connection string and see if it works for you?

For reference, you can see my full test against Azurite here

davidandradeduarte commented 2 years ago

Yep, it works. Thank you @seankane-msft.

I think I grabbed that connection string from the sdk tests, if I'm not mistaken.

This solves the issue and allows us to connect to Azurite using the aztables package, which is what we want 👍

Good work on the docs here!

This is more a nice-to-have rather than anything else, but anyway we would be able to use UseDevelopmentStorage=true (like this)? Or even better, in the previous table sdk we were able to call something like storage.NewEmulatorClient() which would automatically connect to Azurite.

seankane-msft commented 2 years ago

Glad this solved the issue, I see how the test is misleading, hopefully the update to the docs will prevent confusion for future users.

We don't have any plans right now to support a config from like UseDevelopmentStorage=true, this feels more like a C# feature than a Go feature. We also don't plan on adding a NewEmulatorClient method, but I will bring it up during our team meetings.

davidandradeduarte commented 2 years ago

All right, thanks anyway! Closing this issue.

davidandradeduarte commented 2 years ago

Hi, 👋

Reoping this issue, since I've only got the chance to get back to this now.

While migrating to the new SDK, I found that we still have an issue with Go when using batch mode with Azurite.

The returned error is unexpected EOF.

Again, the same code works with a real storage account.

Code can be found here: https://github.com/davidandradeduarte/azurite-insert-batch-bugs/blob/main/go-sdk/14750-aztables/main.go

Edit: Even tho SubmitTransaction errors, it adds ONLY the first batch record (just like reported in the initial issue).

davidandradeduarte commented 2 years ago

@seankane-msft @jhendrixMSFT @ArcturusZhang ^

seankane-msft commented 2 years ago

@davidandradeduarte thanks for pointing this out, I'm investigating.

seankane-msft commented 2 years ago

@davidandradeduarte what version of azurite are you running?

davidandradeduarte commented 2 years ago

@davidandradeduarte what version of azurite are you running?

latest from docker hub

seankane-msft commented 2 years ago

@davidandradeduarte does this error reproduce for you when you target a Storage/Cosmos account? This only occurs for Azurite, which leads me to believe that this issue is which Azurite and not the SDK.

davidandradeduarte commented 2 years ago

@davidandradeduarte does this error reproduce for you when you target a Storage/Cosmos account? This only occurs for Azurite, which leads me to believe that this issue is which Azurite and not the SDK.

Exactly, only happens with Azurite. It works with a real Azure Storage account.

I'm not sure it is an issue with Azurite alone, since batch insert with the C# SDK works (using the same Azurite version), which leads me to think that, in the Go SDK, the Azurite request or response are not being properly handled for batch insert mode.

davidandradeduarte commented 2 years ago

Actually, looking at the Azurite debug logs of a C# batch insert request vs a Go batch insert request, they do look different. Both the request and response.

I've upload the logs to: https://github.com/davidandradeduarte/azurite-insert-batch-bugs/blob/main/azurite-logs/batch-csharp-vs-go.log

Hopefully you have a better understanding of Azurite's API than me @seankane-msft 😄

seankane-msft commented 2 years ago

@davidandradeduarte I opened an issue in the Azurite repository for this bug, I'm not sure why it only reproduces in Go but as far as I can tell the request body is being formed correctly.

https://github.com/Azure/Azurite/issues/1381

davidandradeduarte commented 2 years ago

I've just tested it with the new Azurite build, and works as expected. Closing the issue. Thank you!