Do not truncate aggregate tables on each ingest #841

chakrabortyr · 2019-03-06T21:00:19Z

Description

Motivation and Context

We don't want to be constantly truncating + reingesting potentially millions of rows as cloud usage scales.

Tests performed

Made sure existing tests passed + added new set of anonymised data. This anonymised data is then validated in a post ingest script. I manually did some additional validation with the following queries I couldn't think of a holistic way to operationalise:

This is an old instance with new events in the post ingest. Ran this before and after post_ingest_test.sh to ensure that it got updated with a new end_event/last_modified time:
SELECT * FROM modw_cloud.cloud_events_transient where instance = 'DESIRED_ID';

Then ran this to ensure that we only had two rows where the last_modified timestamp had changed:
SELECT * FROM modw_cloud.cloud_events_transient order by end_time DESC, last_modified DESC;

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project as found in the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

jpwhite4 · 2019-03-07T14:23:51Z

configuration/etl/etl_sql.d/cloud_common/fix-last-modified-columns.sql

@@ -0,0 +1,8 @@
+-- Since we are adding a new 'last_modified' column to event tables, we need to set the 


You do not need to do this. You can re-aggregate by setting the last-modifed time when you run the etl_overseer.

shippable.yml

tests/artifacts/xdmod-test-artifacts/xdmod/referencedata/names.csv

jpwhite4 · 2019-03-07T19:22:10Z

open_xdmod/modules/xdmod/integration_tests/scripts/post_upgrade_ingest.sh

+REF_SOURCE=`realpath $BASEDIR/../../tests/artifacts/xdmod-test-artifacts/xdmod/referencedata`
+REF_DIR=/var/tmp/referencedata
+
+if [ "$XDMOD_TEST_MODE" = "upgrade" ];


You should run the test that adds new data to the system on both the upgrade and fresh install builds.

jpwhite4 · 2019-03-07T19:23:26Z

open_xdmod/modules/xdmod/integration_tests/scripts/post_upgrade_ingest.sh

+then
+    yum -y install ~/rpmbuild/RPMS/*/*.rpm
+    ~/bin/services start
+    expect $BASEDIR/xdmod-upgrade.tcl | col -b


You should not reinstall the RPM or start the servers again or run the upgrade again

jpwhite4 · 2019-03-07T20:43:29Z

Don't forget to update xdmod-ingestor so that it actually sets the last-modified time.

PHP Notice:  Undefined property: ETL\Ingestor\CloudStateReconstructorTransformIngestor::$_last_modified in /usr/share/xdmod/classes/ETL/Ingestor/CloudStateReconstructorTransformIngestor.php on line 135

jpwhite4 · 2019-03-07T20:49:44Z

There is something amiss with the openstack_staging_event table definition. The overseer is Altering it every time it runs:

2019-03-07 19:38:30 [notice] Altering table `modw_cloud`.`openstack_staging_event`

jpwhite4 · 2019-03-07T20:51:06Z

There is something wrong with the jobs-cloud-impor-users-generic pipeline:

2019-03-07 19:29:33 [notice] Start processing section 'xdmod.jobs-cloud-import-users-generic'
2019-03-07 19:29:33 [error] xdmod.jobs-cloud-import-users-generic.GenericCloudStagingUserIngestor (ETL\Ingestor\DatabaseIngestor): SQLSTATE[42S02]: Base table or view not found: 1146 Table 'modw_cloud.generic_cloud_raw_volume' doesn't exist Exception: 'SQLSTATE[42S02]: Base table or view not found: 1146 Table 'modw_cloud.generic_cloud_raw_volume' doesn't exist'
2019-03-07 19:29:33 [warning] Stopping ETL due to exception in xdmod.jobs-cloud-import-users-generic.GenericCloudStagingUserIngestor (ETL\Ingestor\DatabaseIngestor)

@eiffel777 may want to look into this.

jpwhite4 · 2019-03-08T17:00:13Z

configuration/etl/etl_tables.d/cloud_common/event.json

@@ -93,7 +99,8 @@
                "name": "increment_key",
                "columns": [
                    "resource_id",
-                    "event_id"
+                    "event_id",
+                    "last_modified"


jpwhite4 · 2019-03-08T18:44:40Z

classes/ETL/Ingestor/CloudStateReconstructorTransformIngestor.php

@@ -58,6 +59,7 @@ public function __construct(aOptions $options, EtlConfiguration $etlConfig, Log
        $this->_start_event_ids = array(self::START, self::RESUME, self::STATE_REPORT, self::UNSHELVE, self::UNPAUSE, self::UNSUSPEND, self::POWER_ON);
        $this->_all_event_ids = array_merge($this->_start_event_ids, $this->_stop_event_ids);
        $this->_end_time = $etlConfig->getVariableStore()->endDate ? date('Y-m-d H:i:s', strtotime($etlConfig->getVariableStore()->endDate)) : null;
+        $this->_last_modified = date('Y-m-d H:i:s', strtotime('now - 3 minutes'));


jpwhite4 · 2019-03-08T21:38:08Z

classes/ETL/Ingestor/CloudStateReconstructorTransformIngestor.php

-            $this->_last_modified = date('Y-m-d H:i:s', 
-                strtotime($this->getEtlOverseerOptions()->getLastModifiedStartDate()));
+            $this->_last_modified = date(
+                'Y-m-d H:i:s',


jpwhite4 · 2019-03-09T02:42:23Z

diff --git a/classes/ETL/Ingestor/CloudStateReconstructorTransformIngestor.php b/classes/ETL/Ingestor/CloudStateReconstructorTransformIngestor.php
index 761dccb2..c3a3afde 100644
--- a/classes/ETL/Ingestor/CloudStateReconstructorTransformIngestor.php
+++ b/classes/ETL/Ingestor/CloudStateReconstructorTransformIngestor.php
@@ -46,7 +46,6 @@ class CloudStateReconstructorTransformIngestor extends pdoIngestor implements iA
     private $_start_event_ids;
     private $_instance_state;
     private $_end_time;
-    private $_last_modified;

     /**
      * @see ETL\Ingestor\pdoIngestor::__construct()
@@ -59,7 +58,6 @@ class CloudStateReconstructorTransformIngestor extends pdoIngestor implements iA
         $this->_start_event_ids = array(self::START, self::RESUME, self::STATE_REPORT, self::UNSHELVE, self::UNPAUSE, self::UNSUSPEND, self::POWER_ON);
         $this->_all_event_ids = array_merge($this->_start_event_ids, $this->_stop_event_ids);
         $this->_end_time = $etlConfig->getVariableStore()->endDate ? date('Y-m-d H:i:s', strtotime($etlConfig->getVariableStore()->endDate)) : null;
-        $this->_last_modified = null;

         $this->resetInstance();
     }
@@ -129,18 +127,12 @@ class CloudStateReconstructorTransformIngestor extends pdoIngestor implements iA
     protected function getSourceQueryString()
     {
         $sql = parent::getSourceQueryString();
-        if($this->_last_modified === null) {
-            $this->_last_modified = date(
-                'Y-m-d H:i:s',
-                $this->getEtlOverseerOptions()->getLastModifiedStartDate()
-            );
-        }

         // Due to the way the Finite State Machine handles the rows in event reconstruction, the last row
         // is lost. To work around this we add a dummy row filled with zeroes.
         $colCount = count($this->etlSourceQuery->records);
         $unionValues = array_fill(0, $colCount, 0);
-        $subSelect = "(SELECT DISTINCT instance_id from modw_cloud.event WHERE last_modified > \"" . $this->_last_modified . "\")";
+        $subSelect = "(SELECT DISTINCT instance_id from modw_cloud.event WHERE last_modified > \"" . $this->getEtlOverseerOptions()->getLastModifiedStartDate() . "\")";

         $sql = "$sql WHERE instance_id IN " . $subSelect . " AND event_type_id IN (" . implode(',', $this->_all_event_ids) . ")\nUNION ALL\nSELECT " . implode(',', $unionValues) . "\nORDER BY 1 DESC, 2 DESC, 3 ASC, 4 DESC";

smgallo · 2019-03-11T20:02:00Z

open_xdmod/modules/xdmod/regression_tests/post_ingest_test.sh

@@ -11,7 +11,7 @@ sudo -u xdmod xdmod-shredder -r openstack -d $REF_DIR/openstack_upgrade -f opens
 sudo -u xdmod xdmod-ingestor --datatype=openstack --last-modified-start-date "$last_modified_start_date"
 sudo -u xdmod xdmod-ingestor --aggregate=cloud --last-modified-start-date "$last_modified_start_date"

-phpunit="$(readlink -f ../../../../../vendor/bin/phpunit)"
+phpunit="$(readlink -f ../../../../vendor/bin/phpunit)"


Please test these things locally before committing if at all possible. Otherwise, you tie up limited shippable resources.

jpwhite4 · 2019-03-11T20:53:15Z

classes/ETL/Ingestor/CloudStateReconstructorTransformIngestor.php

@@ -58,6 +58,7 @@ public function __construct(aOptions $options, EtlConfiguration $etlConfig, Log
        $this->_start_event_ids = array(self::START, self::RESUME, self::STATE_REPORT, self::UNSHELVE, self::UNPAUSE, self::UNSUSPEND, self::POWER_ON);
        $this->_all_event_ids = array_merge($this->_start_event_ids, $this->_stop_event_ids);
        $this->_end_time = $etlConfig->getVariableStore()->endDate ? date('Y-m-d H:i:s', strtotime($etlConfig->getVariableStore()->endDate)) : null;
+        $this->_last_modified = null;


Please do not add an unused private member variable to the class.

jpwhite4 · 2019-03-11T20:54:24Z

configuration/etl/etl.d/cloud_state_machine.json

@@ -35,7 +35,7 @@
            "name": "cloud-transient",
            "class": "DatabaseIngestor",
            "definition_file": "cloud_common/cloud_transient.json",
-            "truncate_destination": true,
+            "truncate_destination": false,


truncate_destination is set to false by default in lint 4. You do not need to has a tunrcate_destination: false here or on line 49.

configuration/etl/etl_tables.d/cloud_common/cloud_transient.json

configuration/etl/etl_tables.d/cloud_common/event.json

jpwhite4 · 2019-03-11T20:58:50Z

open_xdmod/modules/xdmod/regression_tests/post_ingest_test.sh

+
+last_modified_start_date=$(date +'%F %T')
+
+sudo -u xdmod xdmod-shredder -r openstack -d $REF_DIR/openstack_upgrade -f openstack --last-modified-start-date "$last_modified_start_date"


The xdmod-shredder command does not accept a --last-modified-start-date commandline parameter. Please explain why you added the parameter.

jpwhite4 · 2019-03-11T21:01:58Z

The new test data that you added has overlapping instance_ids but the users and projects for those instances are different in the new data vs the old data. As far as I am aware, the current code is not able to handle this scenario to correctly track user / project changes for an instance. Please can you confirm whether this is/isn't the case.

jpwhite4 · 2019-03-11T21:07:01Z

configuration/etl/etl_action_defs.d/cloud_common/cloud_transient.json

@@ -23,7 +23,8 @@
            "wallduration": "FLOOR(e.end_time_ts) - FLOOR(e.start_time_ts)",
            "person_id": "ev.person_id",
            "systemaccount_id": "ev.systemaccount_id",
-            "submission_venue_id": "ev.submission_venue_id"
+            "submission_venue_id": "ev.submission_venue_id",
+            "last_modified": "ev.last_modified"


you should not set the last_modified time from ev.last_modified since this will correspond to the modification time of the start event for the session. This start event will not have been modified. Instead you should not be setting last_modified explicitly at all and should rely on the database setting it.

jpwhite4 · 2019-03-11T21:12:29Z

Don't forget that you should add tests to cover two different scenarios:

an existing session that was active in the original data and has new events in the new data.
a brand new session (that was not active in the original data).

You then need to confirm that

the existing row in the cloud_events_transient table corresponding to the existing session (1) has been updated correctly with the new end_time_ts value
a new row was added to the cloud_events_transient table for session scenario (2)
only the days/months/quarters/years that have new data are getting re-aggregated in the aggregation step.

I suggest that it will be much more manageable to use a small set of test data (you only need one event for scenario (1) and two events for scenario (2)).

open_xdmod/modules/xdmod/regression_tests/post_ingest_test.sh

jpwhite4 · 2019-03-15T19:08:30Z

The tests you have added do not actually confirm that only the rows that need to change are being updated - i.e. these tests would pass if the whole database were reingested. Please can you explain what testing you did to confirm that only the subset of rows are being changed.

jpwhite4 · 2019-03-15T19:12:35Z

There are errors like this in the logs:

PHP Warning:  Missing argument 1 for OpenXdmod\DataWarehouseInitializer::ingestCloudDataOpenStack(), called in /usr/share/xdmod/classes/OpenXdmod/DataWarehouseInitializer.php on line 130 and defined in /usr/share/xdmod/classes/OpenXdmod/DataWarehouseInitializer.php on line 205
2019-03-15 19:03:50 [notice] Ingesting OpenStack event log data
PHP Notice:  Undefined variable: lastModifiedStartDate in /usr/share/xdmod/classes/OpenXdmod/DataWarehouseInitializer.php on line 213

jpwhite4 · 2019-03-15T19:15:25Z

There is something wrong with the cloud_events_transient table definition:

2019-03-15 19:06:00 [notice] Altering table `modw_cloud`.`cloud_events_transient`

the ETL is trying to alter the table even though it does not need altering.

chakrabortyr · 2019-03-15T19:20:13Z

Github is being weird and not letting me edit my PR description. To ensure it was doing what I expected I ran the following in my Docker:

This is an old instance with new events in the post ingest. Ran this before and after post_ingest_test.sh to ensure that it got updated with a new end_event/last_modified time:
SELECT * FROM modw_cloud.cloud_events_transient where instance = 'DESIRED_ID';

Then ran this to ensure that we only had two rows where the last_modified timestamp had changed:
SELECT * FROM modw_cloud.cloud_events_transient order by end_time DESC, last_modified DESC;

smgallo · 2019-03-15T19:20:27Z

Turn on debug and it should tell you why it thinks the table should be altered as per #807

configuration/etl/etl_tables.d/cloud_common/cloud_transient.json

…bortyr/xdmod into RC_DontTruncateAggregates

classes/OpenXdmod/DataWarehouseInitializer.php

chakrabortyr · 2019-03-15T20:26:20Z

The Alter table issue looks like a preexisting condition:

2019-03-15 20:17:48 [notice] Altering table `modw_cloud`.`cloud_events_transient`
2019-03-15 20:17:48 [debug] Alter table SQL ('Cloud DB', class=ETL\DataEndpoint\Mysql, config=datawarehouse, schema=modw_cloud, host=localhost:3306, user=xdmod):
ALTER TABLE `modw_cloud`.`cloud_events_transient`
CHANGE COLUMN `start_time_ts` `start_time_ts` bigint(18) NULL ;

jpwhite4

Looks good to me.

smgallo · 2019-03-15T21:02:13Z

What is the reason provided by the ETL as to why it's altering the transient table? You should see something like this in the logs:

2019-02-19 20:38:03 [debug] Discover table 'modw.job_tasks'
2019-02-19 20:38:03 [debug] Column local_job_id_raw: values for "type" differ ("int(11)" != "int(11) unsigned")

chakrabortyr · 2019-03-16T02:42:24Z

I'd have to go check the logs to find the specifics; but start_time_ts is already defined as a nullable bigint of size 18. It's odd, but fairly certain it's not related to the PR so I was hoping to take care of it in its own request.

smgallo · 2019-03-16T13:09:21Z

@chakrabortyr Check the logs and provide the requested information. We can address it later but I want to know what the issue is.

chakrabortyr · 2019-03-18T13:33:34Z

2019-03-18 13:31:33 [info] start (action_name: xdmod.cloud-state-pipeline.cloud-transient, action: xdmod.cloud-state-pipeline.cloud-transient (ETL\Ingestor\DatabaseIngestor), start_date: , end_date: )
2019-03-18 13:31:33 [debug] Discover table 'modw_cloud.cloud_events_transient'
2019-03-18 13:31:33 [debug] Column start_time_ts: values for "default" differ ("null" != "0")
2019-03-18 13:31:33 [notice] Altering table `modw_cloud`.`cloud_events_transient`

* Do not truncate aggregate table

jpwhite4 reviewed Mar 7, 2019

View reviewed changes

shippable.yml Outdated Show resolved Hide resolved

jpwhite4 reviewed Mar 7, 2019

View reviewed changes

tests/artifacts/xdmod-test-artifacts/xdmod/referencedata/names.csv Outdated Show resolved Hide resolved

jpwhite4 reviewed Mar 7, 2019

View reviewed changes

chakrabortyr force-pushed the RC_DontTruncateAggregates branch 2 times, most recently from af60ab9 to ef7709e Compare March 7, 2019 21:05

jpwhite4 reviewed Mar 8, 2019

View reviewed changes

smgallo reviewed Mar 11, 2019

View reviewed changes

jpwhite4 reviewed Mar 11, 2019

View reviewed changes

configuration/etl/etl_tables.d/cloud_common/cloud_transient.json Show resolved Hide resolved

jpwhite4 reviewed Mar 11, 2019

View reviewed changes

configuration/etl/etl_tables.d/cloud_common/cloud_transient.json Show resolved Hide resolved

jpwhite4 reviewed Mar 11, 2019

View reviewed changes

configuration/etl/etl_tables.d/cloud_common/event.json Show resolved Hide resolved

jpwhite4 reviewed Mar 11, 2019

View reviewed changes

chakrabortyr force-pushed the RC_DontTruncateAggregates branch 2 times, most recently from e78ac76 to 808e01b Compare March 14, 2019 22:06

smgallo suggested changes Mar 15, 2019

View reviewed changes

open_xdmod/modules/xdmod/regression_tests/post_ingest_test.sh Outdated Show resolved Hide resolved

open_xdmod/modules/xdmod/regression_tests/post_ingest_test.sh Outdated Show resolved Hide resolved

open_xdmod/modules/xdmod/regression_tests/post_ingest_test.sh Outdated Show resolved Hide resolved

chakrabortyr force-pushed the RC_DontTruncateAggregates branch from cf15f76 to fc9e8f0 Compare March 15, 2019 18:40

Do not truncate aggregate table

55aed11

chakrabortyr force-pushed the RC_DontTruncateAggregates branch from bea9d11 to 55aed11 Compare March 15, 2019 18:43

jpwhite4 reviewed Mar 15, 2019

View reviewed changes

configuration/etl/etl_tables.d/cloud_common/cloud_transient.json Outdated Show resolved Hide resolved

chakrabortyr added 2 commits March 15, 2019 15:36

partial cleanup

2b8c55e

Merge branch 'RC_DontTruncateAggregates' of https://github.com/chakra…

e778477

…bortyr/xdmod into RC_DontTruncateAggregates

smgallo suggested changes Mar 15, 2019

View reviewed changes

classes/OpenXdmod/DataWarehouseInitializer.php Outdated Show resolved Hide resolved

chakrabortyr added 3 commits March 15, 2019 15:51

more cleanup

6f94254

sigh

34a680b

no longer add one line

89b7928

made the start/stop times make actual sense

deb991e

jpwhite4 approved these changes Mar 15, 2019

View reviewed changes

smgallo approved these changes Mar 16, 2019

View reviewed changes

chakrabortyr merged commit eed464d into ubccr:xdmod8.1 Mar 18, 2019

chakrabortyr deleted the RC_DontTruncateAggregates branch March 18, 2019 13:35

jtpalmer pushed a commit to jtpalmer/xdmod that referenced this pull request Mar 19, 2019

No longer truncate aggregate tables (ubccr#841)

5c0f2dc

* Do not truncate aggregate table

plessbd added this to the 8.1.0 milestone Mar 23, 2019

chakrabortyr added Category:Cloud Cloud Realm enhancement Enhancement of the functionality of an existing feature labels Mar 26, 2019

jpwhite4 mentioned this pull request Mar 26, 2019

Updated Cloud Documentation for 8.1 Release #875

Merged

6 tasks

jtpalmer mentioned this pull request Apr 2, 2019

Improve storage aggregation #882

Merged

6 tasks

smgallo added the Category:ETL Extract Transform Load label Apr 10, 2019

smgallo changed the title ~~No longer truncate aggregate tables~~ Do not truncate aggregate tables on each ingest Apr 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not truncate aggregate tables on each ingest #841

Do not truncate aggregate tables on each ingest #841

chakrabortyr commented Mar 6, 2019 •

edited by smgallo

Loading

jpwhite4 Mar 7, 2019

jpwhite4 Mar 7, 2019 •

edited

Loading

jpwhite4 Mar 7, 2019

jpwhite4 commented Mar 7, 2019

jpwhite4 commented Mar 7, 2019

jpwhite4 commented Mar 7, 2019

jpwhite4 Mar 8, 2019

jpwhite4 Mar 8, 2019

jpwhite4 Mar 8, 2019

jpwhite4 commented Mar 9, 2019

smgallo Mar 11, 2019

jpwhite4 Mar 11, 2019

jpwhite4 Mar 11, 2019

jpwhite4 Mar 11, 2019

jpwhite4 commented Mar 11, 2019

jpwhite4 Mar 11, 2019 •

edited

Loading

jpwhite4 commented Mar 11, 2019

jpwhite4 commented Mar 15, 2019

jpwhite4 commented Mar 15, 2019

jpwhite4 commented Mar 15, 2019

chakrabortyr commented Mar 15, 2019 •

edited

Loading

smgallo commented Mar 15, 2019

chakrabortyr commented Mar 15, 2019

jpwhite4 left a comment

smgallo commented Mar 15, 2019

chakrabortyr commented Mar 16, 2019

smgallo commented Mar 16, 2019

chakrabortyr commented Mar 18, 2019

		@@ -0,0 +1,8 @@
		-- Since we are adding a new 'last_modified' column to event tables, we need to set the


		last_modified_start_date=$(date +'%F %T')

		sudo -u xdmod xdmod-shredder -r openstack -d $REF_DIR/openstack_upgrade -f openstack --last-modified-start-date "$last_modified_start_date"

Do not truncate aggregate tables on each ingest #841

Do not truncate aggregate tables on each ingest #841

Conversation

chakrabortyr commented Mar 6, 2019 • edited by smgallo Loading

Description

Motivation and Context

Tests performed

Types of changes

Checklist:

Choose a reason for hiding this comment

jpwhite4 Mar 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpwhite4 commented Mar 7, 2019

jpwhite4 commented Mar 7, 2019

jpwhite4 commented Mar 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpwhite4 commented Mar 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpwhite4 commented Mar 11, 2019

jpwhite4 Mar 11, 2019 • edited Loading

Choose a reason for hiding this comment

jpwhite4 commented Mar 11, 2019

jpwhite4 commented Mar 15, 2019

jpwhite4 commented Mar 15, 2019

jpwhite4 commented Mar 15, 2019

chakrabortyr commented Mar 15, 2019 • edited Loading

smgallo commented Mar 15, 2019

chakrabortyr commented Mar 15, 2019

jpwhite4 left a comment

Choose a reason for hiding this comment

smgallo commented Mar 15, 2019

chakrabortyr commented Mar 16, 2019

smgallo commented Mar 16, 2019

chakrabortyr commented Mar 18, 2019

chakrabortyr commented Mar 6, 2019 •

edited by smgallo

Loading

jpwhite4 Mar 7, 2019 •

edited

Loading

jpwhite4 Mar 11, 2019 •

edited

Loading

chakrabortyr commented Mar 15, 2019 •

edited

Loading