Skip to content

Add "off" time using Dean #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
calufa wants to merge 5 commits into
base: feature/add-unit-test-suite
Choose a base branch
from
Open

Add "off" time using Dean #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49f6d87
format sql
calufa Jan 12, 2022
a7ec67f
calc off-time
calufa Jan 15, 2022
dcb3231
optimize query
calufa Jan 17, 2022
63dd258
add off-time to replybot
calufa Jan 17, 2022
e2efd91
add ERROR and WAIT_EXTERNAL_EVENT
calufa Jan 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions dean/dean.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,12 @@ func getConn(cfg *Config) *pgxpool.Pool {

func getQueries(cfg *Config, pool *pgxpool.Pool) []<-chan *ExternalEvent {
lookup := map[string]Query{
"respondings": Respondings,
"blocked": Blocked,
"errored": Errored,
"timeouts": Timeouts,
"followups": FollowUps,
"offtime": OffTime,
"respondings": Respondings,
"timeouts": Timeouts,
}
queries := strings.Split(cfg.Queries, ",")
chans := []<-chan *ExternalEvent{}
Expand Down
145 changes: 94 additions & 51 deletions dean/queries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,84 +73,127 @@ func getFollowUp(rows pgx.Rows) *ExternalEvent {
return &ExternalEvent{userid, pageid, &Event{"follow_up", &value}}
}

func Respondings(cfg *Config, conn *pgxpool.Pool) <-chan *ExternalEvent {
query := `SELECT userid, pageid
FROM states
WHERE
current_state = 'RESPONDING' AND
updated + ($1)::INTERVAL > $3 AND
($3 - updated) > ($2)::INTERVAL`
func getTimeOff(rows pgx.Rows) *ExternalEvent {
var userid, pageid string
err := rows.Scan(&userid, &pageid)
handle(err)

return &ExternalEvent{userid, pageid, &Event{"time_off", nil}}
}

func Respondings(cfg *Config, conn *pgxpool.Pool) <-chan *ExternalEvent {
query := `
SELECT userid, pageid
FROM states
WHERE
current_state = 'RESPONDING' AND
updated + ($1)::INTERVAL > $3 AND
($3 - updated) > ($2)::INTERVAL
`
d := time.Now().UTC()
return get(conn, getRedo, query, cfg.RespondingInterval, cfg.RespondingGrace, d)
}

func Errored(cfg *Config, conn *pgxpool.Pool) <-chan *ExternalEvent {

query := `SELECT userid, pageid
FROM states
WHERE
current_state = 'ERROR' AND
error_tag = ANY($1) AND
updated + ($2)::INTERVAL > $3 AND
($3 > next_retry OR next_retry IS NULL)`

query := `
SELECT userid, pageid
FROM states
WHERE
current_state = 'ERROR' AND
error_tag = ANY($1) AND
updated + ($2)::INTERVAL > $3 AND
($3 > next_retry OR next_retry IS NULL)
`
d := time.Now().UTC()
return get(conn, getRedo, query, cfg.ErrorTags, cfg.ErrorInterval, d)
}

func Blocked(cfg *Config, conn *pgxpool.Pool) <-chan *ExternalEvent {

query := `SELECT userid, pageid
FROM states
WHERE
current_state = 'BLOCKED' AND
fb_error_code = ANY($1) AND
updated + ($2)::INTERVAL > $3 AND
($3 > next_retry OR next_retry IS NULL)`

query := `
SELECT userid, pageid
FROM states
WHERE
current_state = 'BLOCKED' AND
fb_error_code = ANY($1) AND
updated + ($2)::INTERVAL > $3 AND
($3 > next_retry OR next_retry IS NULL)
`
d := time.Now().UTC()
return get(conn, getRedo, query, cfg.Codes, cfg.BlockedInterval, d)
}

func Timeouts(cfg *Config, conn *pgxpool.Pool) <-chan *ExternalEvent {
query := `SELECT (state_json->>'waitStart')::int, userid, pageid
FROM states
WHERE
current_state = 'WAIT_EXTERNAL_EVENT' AND
timeout_date < $1`

query := `
SELECT (state_json->>'waitStart')::int, userid, pageid
FROM states
WHERE
current_state = 'WAIT_EXTERNAL_EVENT' AND
timeout_date < $1
`
d := time.Now().UTC()
return get(conn, getTimeout, query, d)
}

// TODO: test cockroach perf and index
func FollowUps(cfg *Config, conn *pgxpool.Pool) <-chan *ExternalEvent {
query := `WITH x AS
(WITH t AS
(SELECT state_json->>'question' as question, states.userid, states.pageid, surveys.shortcode, has_followup, surveys.created
FROM states
INNER JOIN credentials c
ON pageid = facebook_page_id
INNER JOIN surveys
ON states.current_form = surveys.shortcode
AND c.userid = surveys.userid
WHERE
query := `
WITH x AS (
WITH t AS (
SELECT state_json->>'question' as question, states.userid, states.pageid, surveys.shortcode, has_followup, surveys.created
FROM states
INNER JOIN credentials c ON
pageid = facebook_page_id
INNER JOIN surveys ON
states.current_form = surveys.shortcode AND
c.userid = surveys.userid
WHERE
surveys.created <= form_start_time AND
current_state = 'QOUT' AND
previous_is_followup = FALSE AND
previous_with_token = FALSE AND
(NOW() - updated) > ($1)::INTERVAL AND
(NOW() - updated) < ($2)::INTERVAL
)
SELECT *, ROW_NUMBER() OVER (PARTITION BY userid, pageid, shortcode ORDER BY created DESC)
FROM t
)
SELECT question, userid, pageid
FROM x
WHERE
row_number = 1 AND
has_followup = TRUE`

)
SELECT *, ROW_NUMBER() OVER (
PARTITION BY userid, pageid, shortcode
ORDER BY created DESC
) FROM t
)
SELECT question, userid, pageid
FROM x
WHERE
row_number = 1 AND
has_followup = TRUE
`
return get(conn, getFollowUp, query, cfg.FollowUpMin, cfg.FollowUpMax)
}

func OffTime(cfg *Config, conn *pgxpool.Pool) <-chan *ExternalEvent {
query := `
WITH y AS (
WITH x AS (
SELECT
current_state, pageid, userid
FROM states
WHERE
current_state = 'BLOCKED' OR
current_state = 'ERROR' OR
current_state = 'QOUT' OR
current_state = 'WAIT_EXTERNAL_EVENT'
)
SELECT
responses.pageid, responses.surveyid, responses.userid,
surveys_metadata.off_date, current_state
FROM x
INNER JOIN responses ON
responses.pageid = x.pageid AND
responses.userid = x.userid
INNER JOIN surveys_metadata ON
surveys_metadata.surveyid = responses.surveyid
WHERE surveys_metadata.off_date < NOW()
)
SELECT userid, pageid
FROM y;
`
return get(conn, getTimeOff, query)

@calufa calufa Jan 17, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the query performance using EXPLAIN:

root@:26257/chatroach> 

EXPLAIN WITH x AS (
    SELECT
            responses.pageid, responses.userid, states.current_state
    FROM responses
    INNER JOIN surveys_metadata ON
            surveys_metadata.surveyid = responses.surveyid
    INNER JOIN states ON
            states.pageid = responses.pageid AND
            states.userid = responses.userid
    WHERE off_date < NOW()
)
SELECT userid, pageid
FROM x
WHERE
    current_state = 'QOUT' OR
    current_state = 'BLOCKED';

                 tree                 |       field        |                                                                                                                                    description
--------------------------------------+--------------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
                                      | distributed        | false
                                      | vectorized         | false
  root                                |                    |
   ├── render                         |                    |
   │    └── filter                    |                    |
   │         │                        | filter             | (current_state = 'QOUT') OR (current_state = 'BLOCKED')
   │         └── scan buffer node     |                    |
   │                                  | label              | buffer 1 (x)
   └── subquery                       |                    |
        │                             | id                 | @S1
        │                             | original sql       | SELECT responses.pageid, responses.userid, states.current_state FROM responses INNER JOIN surveys_metadata ON surveys_metadata.surveyid = responses.surveyid INNER JOIN states ON (states.pageid = responses.pageid) AND (states.userid = responses.userid) WHERE off_date < now()
        │                             | exec mode          | all rows
        └── buffer node               |                    |
             │                        | label              | buffer 1 (x)
             └── render               |                    |
                  └── hash-join       |                    |
                       │              | type               | inner
                       │              | equality           | (pageid, userid) = (pageid, userid)
                       │              | left cols are key  |
                       ├── scan       |                    |
                       │              | table              | states@states_current_state_updated_idx
                       │              | spans              | FULL SCAN
                       └── merge-join |                    |
                            │         | type               | inner
                            │         | equality           | (surveyid) = (surveyid)
                            │         | right cols are key |
                            │         | mergeJoinOrder     | +"(surveyid=surveyid)"
                            ├── scan  |                    |
                            │         | table              | responses@responses_surveyid_userid_timestamp_question_ref_idx
                            │         | spans              | FULL SCAN
                            └── scan  |                    |
                                      | table              | surveys_metadata@primary
                                      | spans              | FULL SCAN
                                      | filter             | off_date < now()
(34 rows)

Time: 25.911ms

root@:26257/chatroach>

@calufa calufa Jan 17, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed an more performant alternative, here is the result from EXPLAIN:

root@:26257/chatroach> 
EXPLAIN WITH y AS (
        WITH x AS (
                SELECT
                        current_state, pageid, userid
                FROM states
                WHERE
                        current_state = 'QOUT' OR
                        current_state = 'BLOCKED'
        )
        SELECT
                responses.pageid, responses.surveyid, responses.userid,
                surveys_metadata.off_date, current_state
        FROM x
        INNER JOIN responses ON
                responses.pageid = x.pageid AND
                responses.userid = x.userid
        INNER JOIN surveys_metadata ON
                surveys_metadata.surveyid = responses.surveyid
        WHERE surveys_metadata.off_date < NOW()
)
SELECT userid, pageid
FROM y;
                 tree                 |       field        |                                                                                                                                                                                                                       description
--------------------------------------+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
                                      | distributed        | false
                                      | vectorized         | false
  root                                |                    |
   ├── render                         |                    |
   │    └── scan buffer node          |                    |
   │                                  | label              | buffer 2 (y)
   └── subquery                       |                    |
        │                             | id                 | @S1
        │                             | original sql       | WITH x AS (SELECT current_state, pageid, userid FROM states WHERE (current_state = 'QOUT') OR (current_state = 'BLOCKED')) SELECT responses.pageid, responses.surveyid, responses.userid, surveys_metadata.off_date, current_state FROM x INNER JOIN responses ON (responses.pageid = x.pageid) AND (responses.userid = x.userid) INNER JOIN surveys_metadata ON surveys_metadata.surveyid = responses.surveyid WHERE surveys_metadata.off_date < now()
        │                             | exec mode          | all rows
        └── buffer node               |                    |
             │                        | label              | buffer 2 (y)
             └── render               |                    |
                  └── hash-join       |                    |
                       │              | type               | inner
                       │              | equality           | (pageid, userid) = (pageid, userid)
                       │              | left cols are key  |
                       ├── render     |                    |
                       │    └── scan  |                    |
                       │              | table              | states@states_current_state_updated_idx
                       │              | spans              | /"BLOCKED"-/"BLOCKED"/PrefixEnd /"QOUT"-/"QOUT"/PrefixEnd
                       └── merge-join |                    |
                            │         | type               | inner
                            │         | equality           | (surveyid) = (surveyid)
                            │         | right cols are key |
                            │         | mergeJoinOrder     | +"(surveyid=surveyid)"
                            ├── scan  |                    |
                            │         | table              | responses@responses_surveyid_userid_timestamp_question_ref_idx
                            │         | spans              | FULL SCAN
                            └── scan  |                    |
                                      | table              | surveys_metadata@primary
                                      | spans              | FULL SCAN
                                      | filter             | off_date < now()
(33 rows)

Time: 20.187ms

root@:26257/chatroach>

@nandanrao nandanrao Jan 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple thoughts:

  1. I think more states should be included (i.e. ERROR state, no?) - shouldn't it just be every state except RESPONDING and OFF?

  2. This either assumes everyone only answers one survey or just creates off events for every survey the user ever answered? I like the latter. But there is an issue where if someone is technically in a particular form, but never responded. They will then be able to respond and complete the form later, even if the survey is off (no good!) -- but the off time, being something that is survey-specific (not form-specific), we don't need the actual surveyid. We can use the shortcodes in the state to get all the surveys that the person ever took and do the same thing (or just get the LAST shortcode, already available as current_form, and use that.

@calufa calufa Jan 18, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think more states should be included (i.e. ERROR state, no?) - shouldn't it just be every state except RESPONDING and OFF?

Sure.

The implementation is based on our convo where we only mention QOUT and BLOCKED, https://curiouslearning.slack.com/archives/D02CDGM9DQB/p1640828961004900?thread_ts=1640792174.000200&cid=D02CDGM9DQB, but I am happy to include all the others except RESPONDING and OFF.

@nandanrao nandanrao Jan 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, it requires some decision-making, but that's a trivial change to make later so I'm not overly concerned if we don't get it right on the first try. My instinct is to make more offs rather than less offs at first though!

@nandanrao nandanrao Jan 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(NOT moving to OFF state when the survey is off seems to me like an exception and we can solve that problem as we see it become a problem)

@calufa calufa Jan 19, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ERROR and WAIT_EXTERNAL_EVENT, see e2efd91

}
Loading