Data inconsistencies

[robertorodes]

Hi there,

The point is that I’ve seen no trace of database ACID transactions usage or any other mechanism to ensure data is always (or eventually) consistent when performing write operations, neither in WordPress nor in Woocommerce (except for processing woocommerce orders). This seems to me like a general and critical issue for any Woocommerce based development.

Since it’s pretty usual to find use cases in which a user request triggers multiple write operations, this might eventually lead to a broken database and unpredictable application behavior at some point.

Does woocommerce guarantee that data will always be consistent when performing operations such as creating or editing a product, registering an order, etc.? Am I missing something?

Otherwise, do you have any plans on this matter?

[claudiulodro]

Generally the only time this has been an issue is for very high-traffic sites where there's a chance of multiple people checking out or doing something at the exact same time. You need to be getting multiple orders a second for this to really be a problem.

#16971 has information about what we're doing to address the issue. We're going to use the queue in areas where it's needed. The first trial-run implementation is in the new image regeneration feature and if it goes well there we'll be expanding to other areas of the codebase. Thoughts and feedback are welcome at #16971.

[robertorodes]

@claudiulodro Thanks for your quick reply. I see your point. However, if I'm not wrong, what you are dealing with on issue #16971 relates to concurrency (which is another really interesting topic by the way) and even though it might help with data consistency if an event was the only information stored as the result of a user request (it would be an atomic write), I don't know if that would be possible without using ACID transactions, considering that other write operations might be triggered on the database as the result of a user request, either by wordpress, woocommerce itself or by any other plugin hooked into it.

Just to make things more clear about what I'm talking about, let me explain it through an example (based on Woocommerce 3.2.6).

If we have a look at WC_Order_Data_Store_CPT which seems to be the data layer used to update and load orders from the database, we can see that in the different update functions available there, such as create (line 75) or update (line 142), values returned from the write functions used underneath seem to be completely ignored, and thus, no return value is provided to callers of these functions. Taking into account that they trigger multiple write operations on the db, here we might have an scenario in which an error might occur in the middle of the request and some of the writes might not be persisted (while others in fact might do it), leaving the database in an inconsistent and unpredictable state.

I think that's ok since the transaction, from my point of view, should be managed by callers in upper layers. However, the main problem I see here is that error cases are not properly reported to callers, leaving them without enough knowledge to notice whether the operation has been executed successfully or not (which is a requisite to decide whether to commit or rollback the transaction, provide feedback to users and/or log the failure).

This example can be found as well in other areas of Wordpress (something similar happens in core API functions such as 'wp_insert_post') and Woocommerce (e.g. in products).

Maybe I am missing something but if I'm right, this could lead to the mentioned inconsistent state of the database and unpredictable behavior of the application.

Notice that the difference here with respect to the issue you point to, is that for an error to happen, we would not necesarilly require high load on the server since even a simple database server outage in the middle of a user request might make this occur.

Is there anything I've overlooked? Comments or ideas?

[claudiulodro]

Reopening this so I can do some more research into it.

cc. @kloon @coderkevin I believe you are familiar with these sorts of things, if you have a good answer why this isn't a problem or what sort of change we'd have to implement.

[mikejolley]

Since it’s pretty usual to find use cases in which a user request triggers multiple write operations, this might eventually lead to a broken database and unpredictable application behavior at some point.

Yes in theory this can happen.

Right now we're relying on WP functions etc to save data (e.g wp_update_post) which can fire actions 3rd parties can use. If we implemented a transaction here (around our save method), we could see non-write actions happening before the transaction completes and which, if the write eventually failed, would lead to undesired results.

Example:

1. save order.
2. webhook sends out when 'post' is saved.
3. something throws error.
4. transaction gets rolled back.
5. The data never persisted but the webhook sent the data anyway.

So long as we don't control everything db related, these anomalies will exist. So to answer:

Otherwise, do you have any plans on this matter?

Yes, transactions will be implemented when we move to custom tables and control all DB writes, actions, and events.

[mikejolley]

For ref, this is for orders #10071 and we have a separate project running for products. Both moving to custom tables.

[robertorodes]

Thanks @mikejolley and @claudiulodro for the clarification.