Skip to content

Conversation

@hikinggrass
Copy link
Member

Otherwise it might happen that we try to insert metervalues into the database while another transaction event is happening. In certain circumstances (for example with multiple EVSEs present) this can lead to a "Could not begin transaction" exception

Describe your changes

Issue ticket number and link

Checklist before requesting a review

throw std::runtime_error("Table does not exist.");
}

std::scoped_lock lock(this->database_transaction_mutex);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to do this more generically by putting this in the database connection somehow. Apparently you can't have more than 1 transaction per connection so that seems to be the perfect place. Lock on begin transaction and unlock on the commit or rollback.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that sounds like a good idea!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, we discussed this morning I will pick this one up ;)

@Pietfried Pietfried self-assigned this Apr 29, 2024
@marcemmers marcemmers force-pushed the bugfix/database-transactions branch from 577f803 to e72168f Compare May 2, 2024 09:36
@Pietfried Pietfried marked this pull request as draft May 6, 2024 09:42
hikinggrass and others added 3 commits May 7, 2024 16:36
Otherwise it might happen that we try to insert metervalues into the database while another transaction event is happening.
In certain circumstances (for example with multiple EVSEs present) this can lead to a "Could not begin transaction" exception

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
@marcemmers marcemmers force-pushed the bugfix/database-transactions branch from e72168f to 024aba6 Compare May 7, 2024 14:36
@marcemmers marcemmers marked this pull request as ready for review May 7, 2024 15:03
Co-authored-by: Piet Gömpel <37657534+Pietfried@users.noreply.github.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
@marcemmers marcemmers merged commit 5278e16 into main May 13, 2024
christopher-davis-afs pushed a commit to US-JOET/libocpp that referenced this pull request May 30, 2024
* Guard 2.0.1 database transactions with a mutex

Otherwise it might happen that we try to insert metervalues into the database while another transaction event is happening.
In certain circumstances (for example with multiple EVSEs present) this can lead to a "Could not begin transaction" exception

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

* Add transaction class handling the locking of the interface

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Fix small issues

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Update include/ocpp/common/database/database_connection.hpp

Co-authored-by: Piet Gömpel <37657534+Pietfried@users.noreply.github.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>

---------

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
Co-authored-by: Marc Emmers <m.emmers@alfen.com>
Co-authored-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
Co-authored-by: Piet Gömpel <37657534+Pietfried@users.noreply.github.com>
drmrd pushed a commit to US-JOET/libocpp that referenced this pull request Jun 4, 2024
* Guard 2.0.1 database transactions with a mutex

Otherwise it might happen that we try to insert metervalues into the database while another transaction event is happening.
In certain circumstances (for example with multiple EVSEs present) this can lead to a "Could not begin transaction" exception

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

* Add transaction class handling the locking of the interface

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Fix small issues

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Update include/ocpp/common/database/database_connection.hpp

Co-authored-by: Piet Gömpel <37657534+Pietfried@users.noreply.github.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>

---------

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
Co-authored-by: Marc Emmers <m.emmers@alfen.com>
Co-authored-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
Co-authored-by: Piet Gömpel <37657534+Pietfried@users.noreply.github.com>
Signed-off-by: Daniel Moore <9156191+drmrd@users.noreply.github.com>
@marcemmers marcemmers deleted the bugfix/database-transactions branch November 18, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants