{"action":"created","issue":{"url":"https://api.github.com/repos/eventespresso/event-espresso-core/issues/306","repository_url":"https://api.github.com/repos/eventespresso/event-espresso-core","labels_url":"https://api.github.com/repos/eventespresso/event-espresso-core/issues/306/labels{/name}","comments_url":"https://api.github.com/repos/eventespresso/event-espresso-core/issues/306/comments","events_url":"https://api.github.com/repos/eventespresso/event-espresso-core/issues/306/events","html_url":"https://github.com/eventespresso/event-espresso-core/issues/306","id":315225216,"node_id":"MDU6SXNzdWUzMTUyMjUyMTY=","number":306,"title":"Implement Conditional Update Queries for Data Fields Requiring Millisecond Accuracy","user":{"login":"tn3rb","id":1751030,"node_id":"MDQ6VXNlcjE3NTEwMzA=","avatar_url":"https://avatars1.githubusercontent.com/u/1751030?v=4","gravatar_id":"","url":"https://api.github.com/users/tn3rb","html_url":"https://github.com/tn3rb","followers_url":"https://api.github.com/users/tn3rb/followers","following_url":"https://api.github.com/users/tn3rb/following{/other_user}","gists_url":"https://api.github.com/users/tn3rb/gists{/gist_id}","starred_url":"https://api.github.com/users/tn3rb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tn3rb/subscriptions","organizations_url":"https://api.github.com/users/tn3rb/orgs","repos_url":"https://api.github.com/users/tn3rb/repos","events_url":"https://api.github.com/users/tn3rb/events{/privacy}","received_events_url":"https://api.github.com/users/tn3rb/received_events","type":"User","site_admin":false},"labels":[{"id":904083603,"node_id":"MDU6TGFiZWw5MDQwODM2MDM=","url":"https://api.github.com/repos/eventespresso/event-espresso-core/labels/category:models-and-data-infrastructure","name":"category:models-and-data-infrastructure","color":"abc2c1","default":false},{"id":904083482,"node_id":"MDU6TGFiZWw5MDQwODM0ODI=","url":"https://api.github.com/repos/eventespresso/event-espresso-core/labels/category:spco-&-registration","name":"category:spco-&-registration","color":"abc2c1","default":false},{"id":904091240,"node_id":"MDU6TGFiZWw5MDQwOTEyNDA=","url":"https://api.github.com/repos/eventespresso/event-espresso-core/labels/priority:high","name":"priority:high","color":"f7d500","default":false},{"id":906240647,"node_id":"MDU6TGFiZWw5MDYyNDA2NDc=","url":"https://api.github.com/repos/eventespresso/event-espresso-core/labels/status:feedback-needed","name":"status:feedback-needed","color":"a16ec6","default":false},{"id":22687539,"node_id":"MDU6TGFiZWwyMjY4NzUzOQ==","url":"https://api.github.com/repos/eventespresso/event-espresso-core/labels/type:enhancement%20%F0%9F%8E%89","name":"type:enhancement 🎉","color":"efefef","default":false}],"state":"open","locked":false,"assignee":{"login":"tn3rb","id":1751030,"node_id":"MDQ6VXNlcjE3NTEwMzA=","avatar_url":"https://avatars1.githubusercontent.com/u/1751030?v=4","gravatar_id":"","url":"https://api.github.com/users/tn3rb","html_url":"https://github.com/tn3rb","followers_url":"https://api.github.com/users/tn3rb/followers","following_url":"https://api.github.com/users/tn3rb/following{/other_user}","gists_url":"https://api.github.com/users/tn3rb/gists{/gist_id}","starred_url":"https://api.github.com/users/tn3rb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tn3rb/subscriptions","organizations_url":"https://api.github.com/users/tn3rb/orgs","repos_url":"https://api.github.com/users/tn3rb/repos","events_url":"https://api.github.com/users/tn3rb/events{/privacy}","received_events_url":"https://api.github.com/users/tn3rb/received_events","type":"User","site_admin":false},"assignees":[{"login":"tn3rb","id":1751030,"node_id":"MDQ6VXNlcjE3NTEwMzA=","avatar_url":"https://avatars1.githubusercontent.com/u/1751030?v=4","gravatar_id":"","url":"https://api.github.com/users/tn3rb","html_url":"https://github.com/tn3rb","followers_url":"https://api.github.com/users/tn3rb/followers","following_url":"https://api.github.com/users/tn3rb/following{/other_user}","gists_url":"https://api.github.com/users/tn3rb/gists{/gist_id}","starred_url":"https://api.github.com/users/tn3rb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tn3rb/subscriptions","organizations_url":"https://api.github.com/users/tn3rb/orgs","repos_url":"https://api.github.com/users/tn3rb/repos","events_url":"https://api.github.com/users/tn3rb/events{/privacy}","received_events_url":"https://api.github.com/users/tn3rb/received_events","type":"User","site_admin":false},{"login":"mnelson4","id":1766382,"node_id":"MDQ6VXNlcjE3NjYzODI=","avatar_url":"https://avatars3.githubusercontent.com/u/1766382?v=4","gravatar_id":"","url":"https://api.github.com/users/mnelson4","html_url":"https://github.com/mnelson4","followers_url":"https://api.github.com/users/mnelson4/followers","following_url":"https://api.github.com/users/mnelson4/following{/other_user}","gists_url":"https://api.github.com/users/mnelson4/gists{/gist_id}","starred_url":"https://api.github.com/users/mnelson4/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mnelson4/subscriptions","organizations_url":"https://api.github.com/users/mnelson4/orgs","repos_url":"https://api.github.com/users/mnelson4/repos","events_url":"https://api.github.com/users/mnelson4/events{/privacy}","received_events_url":"https://api.github.com/users/mnelson4/received_events","type":"User","site_admin":false}],"milestone":null,"comments":9,"created_at":"2018-04-17T20:35:14Z","updated_at":"2018-12-12T18:53:00Z","closed_at":null,"author_association":"MEMBER","body":"Because our data model system is currently following an Active Record pattern, we have been performing separate reads and writes when updating data. \r\nThis can increase the possibility of race conditions having a detrimental effect on the accuracy of our data, and can even lead to our data being outright corrupted.\r\n \r\nconsider the following events happening in the following order:\r\n\r\n- Request A reads value x from the db (x = 2)\r\n- Request A adds 1 to value x and saves it back to the db (now x = 3)\r\n- Request B reads value x from the db (x = 3)\r\n- Request B adds 1 to value x and saves it back to the db (now x = 4)\r\n\r\nMost queries only take a fraction of a second to run, but let's consider a scenario where a server is getting absolutely smashed and run those requests so close together that they are effectively happening at the same time:\r\n\r\n- Request A reads value x from the db (x = 2)\r\n- Request B reads value x from the db (x = 2)\r\n- Request A adds 1 to value x and saves it back to the db (now x = 3)\r\n- Request B adds 1 to value x and saves it back to the db (now x = 3)\r\n\r\nnow imagine that value x is the number of tickets being sold.\r\n\r\nAlthough WPDB does not support it directly, we need to implement UPDATE queries that utilize the existing column data as part of the calculation.\r\nie: \r\n\r\n```sql\r\nUPDATE t1 SET col1 = col1 + 1;\r\n```\r\n\r\nThis would be an improvement over our existing queries because race conditions will not result in data overwrites because the new value is added to the existing column data.\r\n\r\n- Request A reads value x from Col_X in the db (x = 2)\r\n- Request A adds 1 to the value of Col_X (now x = 3)\r\n- Request B reads value x from Col_X in the db (x = 3)\r\n- Request B adds 1 to the value of Col_X (now x = 4)\r\n\r\nThis however still does not prevent race conditions that result in overselling because we can still have the following type of scenario where Col_X should not have been allowed to exceed a value of 3 (ie: 3 spaces available):\r\n\r\n- Request A reads value x from Col_X in the db (x = 2)\r\n- Request B reads value x from Col_X in the db (x = 2)\r\n- Request A adds 1 to the value of Col_X (now x = 3)\r\n- Request B adds 1 to the value of Col_X (now x = 4)\r\n\r\nThese kinds of queries can be further enhanced by implementing conditionals within them that only perform the update if a set of conditions is met.\r\n\r\nFor example:\r\n\r\nThe following query would only update the sold count for the specified ticket, if doing so does not exceed the total quantity available:\r\n\r\n```sql\r\nUPDATE `wp_esp_ticket`\r\nSET `tkt_sold` = \r\nCASE\r\n   WHEN (`tkt_sold` + 4) < `tkt_qty`\r\n   THEN `tkt_sold` + 4\r\n   ELSE `tkt_sold` \r\nEND\r\nWHERE `TKT_ID` = 10;\r\n```\r\n\r\nSo if ticket 10 has a Qty of 25, and already has a sold count of 20, then `( 20 + 4 ) < 25` would equate to true and the update would proceed. \r\nThe exact same query happening a millisecond later however would not be successful because `( 24 + 4 ) < 25` would equate to false.\r\n\r\nOf course, ticket sales are not limited solely by the `tkt_qty` and need to take the ticket's corresponding datetime sold and reg limit fields into consideration, which in turn need to take all other related tickets into consideration, which is an extremely complex set of calculations. So because of this, we will likely need to pre-calculate the ticket availability and then pass that value into the query to do something like:\r\n\r\n```sql\r\nUPDATE `wp_esp_ticket`\r\nSET `tkt_sold` = \r\nCASE\r\n   WHEN (`tkt_sold` + 4) < {QTY}\r\n   THEN `tkt_sold` + 4\r\n   ELSE `tkt_sold` \r\nEND\r\nWHERE `TKT_ID` = 10;\r\n```\r\n\r\nThis of course means we're back to having a race condition because calculating a value for `{QTY}` can not be done within the same query because that calculation is far too complex to be performed in SQL. As I have said before, the only absolutely guaranteed way to prevent a race condition and overselling is to form a single line in the physical world. Switching to conditional update queries will however prevent any kind of data overwrites from occurring and result in greater data accuracy. Ultimately it may be possible to reduce the race conditions even further using transactional queries but this does not necessarily solve the race conditions caused by having to perform any pre-calculations.\r\n\r\nNext steps include:\r\n\r\n - deciding which fields would benefit from this type of query: ie: `tkt_sold`, `tkt_reserved`, `dtt_sold`, `dtt_reserved`\r\n - deciding whether logic for update queries should be added to `EEM_Base` or performed as raw SQL from the respective models (ie: `EEM_Ticket` and `EEM_Datetime`)\r\n - deciding how / where / when pre-calculations will be performed to minimize race conditions\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/eventespresso/event-espresso-core/issues/comments/446700872","html_url":"https://github.com/eventespresso/event-espresso-core/issues/306#issuecomment-446700872","issue_url":"https://api.github.com/repos/eventespresso/event-espresso-core/issues/306","id":446700872,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NjcwMDg3Mg==","user":{"login":"tn3rb","id":1751030,"node_id":"MDQ6VXNlcjE3NTEwMzA=","avatar_url":"https://avatars1.githubusercontent.com/u/1751030?v=4","gravatar_id":"","url":"https://api.github.com/users/tn3rb","html_url":"https://github.com/tn3rb","followers_url":"https://api.github.com/users/tn3rb/followers","following_url":"https://api.github.com/users/tn3rb/following{/other_user}","gists_url":"https://api.github.com/users/tn3rb/gists{/gist_id}","starred_url":"https://api.github.com/users/tn3rb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tn3rb/subscriptions","organizations_url":"https://api.github.com/users/tn3rb/orgs","repos_url":"https://api.github.com/users/tn3rb/repos","events_url":"https://api.github.com/users/tn3rb/events{/privacy}","received_events_url":"https://api.github.com/users/tn3rb/received_events","type":"User","site_admin":false},"created_at":"2018-12-12T18:53:00Z","updated_at":"2018-12-12T18:53:00Z","author_association":"MEMBER","body":"My comment regarding forming a \"single line\" was meant as more of a commentary on the difficulty with preventing race conditions in http requests and not as a desirable goal to aim for. \r\n\r\nThe biggest problem with trying to enforce a \"single line\" approach for Event Espresso, assuming we keep the current default session expiration time of 60 minutes, is that it would reduce the throughput of our registration process down to something like 1 registration per hour. This is just not acceptable and would be a step backwards as far as I'm concerned. Some of the sites we have seen these potential overselling race conditions occurring on have been processing well over 1 registration **per minute**, so enforcing a \"single line\" would result in less than 1/60th of the throughput. Even reducing the session expiration to 15 minutes would only allow a throughput of 4 registrations per hour or 1/15th the throughput.\r\n\r\nSo regardless of whether any of the above actually becomes necessary, I still think it is imperative that we first improve some of our critical queries by implementing update queries that increment stored values directly instead of performing separate reads and writes. To correct for the complexities involved with calculating ticket sale limits due to the interaction of ticket quantities and datetime/venue capacities, we could if need be, perform a secondary read query after any updates to ensure that limits have not been exceeded, but if that has happened, then simply undo the previous update and inform the registrant that their requested ticket quantity is not available.\r\n\r\nIf that alone does not prove to be enough to prevent overselling race conditions on super busy sites, then I would prefer to implement a variation on \"rate limiting\" that would run as a new middleware class in our request stack (see: `/core/services/request/`) but track the number of registrations in progress. The benefit of this approach is that the number of concurrent registrations could be controlled via an admin option and allow customers with less than optimal hosting to reduce the strain on their servers to a level that is more manageable for them. **Then** if an admin wants or needs to, they could reduce the number of concurrent registration allowed to only 1.\r\n\r\nHere's a relatively simple example of a rate limiting request stack middleware class https://github.com/rtheunissen/guzzle-rate-limiter but there are lots of other examples out there that we could use for \"inspiration\".\r\n\r\nThis of course is not too too far off from what you are suggesting, but just implemented in a different manner.\r\n\r\nA rate limiting middleware service could also potentially work in conjunction with what is suggested in #344 "}}
{"actor":{"display_login":"tn3rb"}}