History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: OX-4754
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Critical Critical
Assignee: andrew.hill
Reporter: andrew.hill
Votes: 0
Watchers: 0

If you were logged in you would be able to see more operations.
OpenX Ad Server

Addition of a new zone causes past ZIF data for other zones to be discarded under InnoDB

Created: 16/Jan/09 01:16 PM   Updated: 16/Apr/09 12:55 PM
Component/s: OXP: Database Abstraction: MySQL, OXP: Maintenance Engine
Affects Version/s: OpenX 2.6.3, OpenX 2.7.28-beta
Fix Version/s: Milestone 27, Milestone 28, OpenX 2.7.30-beta
Security Level: Public (All users can see these issues)

Time Tracking:
Original Estimate: 8h
Original Estimate - 8h
Remaining Estimate: 14.1h
Time Spent - 5.75h Remaining Estimate - 14.1h
Time Spent: 5.75h
Time Spent - 5.75h Remaining Estimate - 14.1h

Issue Links:

 All   Comments   Work Log   Change History   FishEye   Crucible   Builds      Sort Order: Ascending order - Click to sort in descending order

Andrew's notes:
  • InnoDB code tries to be clever and do it all in a transaction by starting transaction, deleting old data, inserting new data, completing transaction; instead of the MyISAM approach of SELECT, if present, UPDATE, otherwise, INSERT.
  • Problem is that when a new zone is added to the system, the next MPE run produces ZIF data for the new zone for a whole week; as a result, the min/max start/end dates found in the array of ZIF data are for the whole week, so, the InnoDB DELETE statement deletes the whole week's data.
  • Thus, for the code to work correctly, the whole week's data that is to be deleted needs to be correctly re-inserted; but the code that selects "past zone impression forecasts, so they can be updated with the actual impressions that happened, if possible", although it initially SELECTs the required data, it later only inserts this data into the $aForecasts array if there is already a forecast for the operation interval in the array – and of course, under the above conditions, for all of the operation intervals in the historical week that are before the last MPE run time will not be in the array for pre-existing zones.

Possible solutions:

  1. Update the code to always insert the data SELECTed into the $aForecasts array. May contain side-effects, probably poor efficiency.
  2. Try to implement per-zone updating:
    • Insert guaranteed non-existing new rows using a single bulk if the zone is new;
    • Otherwise attempt an UPDATE, on fail INSERT approach? Can this be improved for performance for recently created zones?
    • Something else?

The following is a patch to an existing integration test that will help demonstrate the issue:
Index: lib/OA/Maintenance/Priority/AdServer/Task/tests/integration/ForecastZoneImpressions.zif.test.php
--- lib/OA/Maintenance/Priority/AdServer/Task/tests/integration/ForecastZoneImpressions.zif.test.php    (revision 31002)
+++ lib/OA/Maintenance/Priority/AdServer/Task/tests/integration/ForecastZoneImpressions.zif.test.php    (working copy)
@@ -175,9 +175,6 @@
         $storedForecasts   = $doData_summary_zone_impression_history->getRowCount();
         $expectedForecasts = OX_OperationInterval::operationIntervalsPerWeek() * 3;
-//        echo "<pre>";
-//        var_dump($storedForecasts);
-//        var_dump($expectedForecasts);
         $this->assertEqual($storedForecasts, $expectedForecasts);
         // For the Zone ID 0 zone and the two "real" zones...
         for ($zoneId = 0; $zoneId < 3; $zoneId++) {
@@ -239,10 +236,33 @@
         $oLogCompletion->oController->oUpdateFinalToDate        = $aDates['end'];

+        // Add a third zone, which will demonstrate OX-4754, in combination
+        // with the "exit" statement later on...
+        $doZones = OA_Dal::factoryDO('zones');
+        $oNow = new Date();
+        $doZones->zonename = 'Third Zone';
+        $doZones->zonetype = 3;
+        $doZones->updated = $oNow->format('%Y-%m-%d %H:%M:%S');
+        $idZoneThird = DataGenerator::generateOne($doZones, true);
+        $doAdZone = OA_Dal::factoryDO('ad_zone_assoc');
+        $doAdZone->ad_id = $idBanner;
+        $doAdZone->zone_id = $idZoneThird;
+        $doAdZone->priority = 0;
+        $doAdZone->link_type = 1;
+        $doAdZone->priority_factor = 1;
+        $doAdZone->to_be_delivered = 1;
+        $idAdZone = DataGenerator::generateOne($doAdZone);
         // NUMBER 3: Run the ZIF task
         $oTask = new OA_Maintenance_Priority_AdServer_Task_ForecastZoneImpressions();

+        // Halt execution here, so that the errors in the DB
+        // can be expected by hand. Where did all the past
+        // data for zone IDs 0, 1 and 2 go?
+        exit;
         // For the Zone ID 0 zone, check that the NUMBER 1 task data is still present

Lukasz Dybcio - 17/Feb/09 12:16 PM
Andrew, is that bug fixed in 2.6 as well? Or only in 2.7?