Fire TableTriggers when an upstream View is updated - #171
Conversation
9a9ef7c to
170bca6
Compare
| yamlApi.createWithAnnotationsAndLabels(yaml, annotations, labels); | ||
| } | ||
|
|
||
| private V1alpha1View getView(V1alpha1TableTrigger trigger) throws SQLException { |
There was a problem hiding this comment.
Is the correlation between objects, in terms of ownership still
View -> Pipeline -> Trigger?
It seems odd to now have a link from the Trigger back to the View
There was a problem hiding this comment.
The View here isn't necessarily the same View as in your example. In the typical View->Pipeline->Trigger situation, the Trigger likely listens to one of the source tables in the View, not the View itself. Or, the Trigger may listen to an upstream View, if you have a views-of-views situation.
I'm planning to add a CREATE TRIGGER DDL along these lines:
CREATE MATERIALIZED VIEW FOO AS SELECT a, b FROM T;
CREATE TRIGGER BAR ON FOO AS 'my-job-template';
In this case, the CREATE...VIEW may result in Triggers, but they would listen to specific tables, not views. The CREATE TRIGGER would create a new Trigger that is wired up to listen to the View object, rather than a set of physical tables. Both leverage JobTemplates and TableTriggers under the hood, but in slightly different ways.
There was a problem hiding this comment.
Great, this makes sense. Seems pretty powerful too, seems like we can define multiple triggers on the same view that may do different things at different cadences or whatnot as we expand the trigger feature set.
| @@ -36,6 +36,14 @@ spec: | |||
| table: | |||
There was a problem hiding this comment.
The table field is currently a required property in this CRD. Now that the view will have its own reconciler, which will handle updates and subsequently update the trigger timestamp in the table trigger object, do we still need the viewRef?
Instead, we could introduce a breaking change by renaming the table property to something more generic, such as source, which could represent either a single table or a view.
There was a problem hiding this comment.
I thought of that, but a view is technically a kind of table (especially if it's materialized), so I think it's reasonable to reuse table here.
There was a problem hiding this comment.
yep, that works! I was mainly thinking that we don’t need a separate view ref.
There was a problem hiding this comment.
Yeah I'm going to drop the viewRef and use a label instead. That way the new ViewReconciler can fire any TableTriggers that have a matching view label.
2669faa to
0b0a326
Compare
626426a to
afa823d
Compare
srnand
left a comment
There was a problem hiding this comment.
looks like one of the integration tests failed ? otherwise lgtm
9d9198b to
24146f1
Compare
24146f1 to
a8f02cb
Compare
| V1alpha1ViewStatus status = object.getStatus(); | ||
| if (status != null && status.getWatermark() != null) { | ||
| log.info("View {} was updated at {}.", name, status.getWatermark()); | ||
| for (V1alpha1TableTrigger trigger : tableTriggerApi.select(VIEW_KEY + "=" + name)) { |
There was a problem hiding this comment.
I imagine this view label will be added by the CREATE TRIGGER statement, right?
There was a problem hiding this comment.
yes that's the plan
Summary
.status.watermarktoViewobjects.ViewReconcilerto detect changes to view watermarks.TableTriggerswhen an upstreamViewis updated.Details
In order to support jobs that listen to multiple tables, we have a few options:
TableTrigger. Instead of one table, listen to an array of tables.Triggertype that supports multiple tables. Maybe call itMultiTableTrigger.View, which supports an arbitrary number of source tables.This PR implements option 3, which is probably the least obvious. The motivation here is to mimic traditional
CREATE TRIGGERDDL, e.g. as supported by Oracle, which works with either tables or views but not lists of tables. In typical DDL, you must firstCREATE VIEW FOOand thenCREATE TRIGGER BAR ON FOO. This means the trigger has a single object to listen to (the view object) rather than a list of objects.In order to support this pattern, I've added a new
ViewRconcilerwhich monitors.status.watermarkand fires any listeningTableTriggers. Batch-oriented jobs that materialize a view can update the view's watermark to trigger downstream jobs. Stream-oriented jobs can do the same whenever they checkpoint.Testing Done
New unit test passes: