Statement::bind_clean_up keeps deleted indicator entries in vector

SOCI / soci

Official repository of the SOCI - The C++ Database Access Library

http://soci.sourceforge.net/

Boost Software License 1.0

1.41k stars 477 forks source link

Statement::bind_clean_up keeps deleted indicator entries in vector #1058

Closed cstiborg closed 1 year ago

cstiborg commented 1 year ago

I discovered that my application became slower and slower at inserting records. After chasing wild geese in my database, I did some rudimentary statistics and my results were that the first 10000 records I could insert in 7 seconds, while the last block of 10000 records up to 2 million records took 2 minutes, and the culprit was Statement::bind_cleanup(). After some more investigation I found that entries in the indicators vector were surely removed, but the entries were never deleted. I did some investigation into why this might be the case, but I didn't manage to find any reason. Finally, I ended up making a simple patch, which clears the vector after its entries are deleted. I've tested the patch in my application and I can't produce any bad behaviour. That's obviously not a mark of quality, particularly given my poor knowledge of the inner workings of SOCI.

soci.patch

zann1x commented 1 year ago

Looking at the method you edited in your patch, it is indeed weird to see that indicators are cleaned differently than the uses and intos above it. Could you share a minimal reproducible example for the issue?

cstiborg commented 1 year ago

Yes - I will create one.

cstiborg commented 1 year ago

I've created a minimal reproducible example and added it to this message:

soci-issue-1058.cpp contains a small program that connects to a postgres database, sets up a prepared statement, and then uses it to add one million records to a database table. For every 10,000 records it prints out how far it has gotten with a timestamp. I think an important part in reproducing is to use the values object to set the input (line 61-65). I haven't been able to reproduce using use(...).
For your debugging pleasure I've added a couple of Dockerfiles which setup a postgresql database and creates a table and role as expected by the minimal reproducible example. (You can use them to see how I build the test environment and how I run it)

To build and run either the original or patched code in docker execute the following commands as root: # cp Dockerfile.[original|patched] Dockerfile # docker build -t soci-issue-1058:latest . # docker run -it soci-issue-1058

soci-issue-1058.tar.gz

zann1x commented 1 year ago

Thanks a lot! I'll look into it.

zann1x commented 1 year ago

After having a look at it, I currently don't see anything else that might be the cause for the degrading performance. The indicators vector is only filled when using the values API, which I'm unfortunately not deeply familiar with either. Nonetheless, the number of (freed) elements in the vector grows indefinitely, so that definitely seems like a bug needing to be fixed. As you have already provided a solution, feel free to create a PR for that!