kesne / jeopardy-bot

:boom: ​A Slack bot for Jeopardy! games. :boom:
31 stars 23 forks source link

Rebuild on Redux #183

Closed kesne closed 6 years ago

kesne commented 8 years ago

Pretty soon, I'll be cutting a branch to migrate this into a big redux-powered app. What this basically means is that instead of a lot of mongoose models and async model methods being called, there will be a set of synchronous reducers that compose the entire state of the application.

This will be a significant breaking change. Once things are finalized, I'll try to make a migration path to the new version, which will almost certainly be a DB dump.

Motivation

How things work right now is a very carefully constructed set of models, glued together with the trebek module, which handles transforming state. Sometimes this happens directly in the commands, and sometimes there are helper methods that do it. None of it is very good though, and the barrier for entry is ridiculously high because untangling how everything works is almost impossible.

The way that messages are constructed and sent is also problematic, because they exist in so many different commands, that ensuring the application responds consistently is pretty difficult.

New Architecture

The database is now a redux store, which is kept entirely in memory. There is a store subscriber that syncs the current state of the store into some persistent data storage. This could either just be a file (which I would prefer to be the standard, but heroku would break :disappointed:) or any sort of DB (mongodb support will likely be bundled by default). Because we really just need read/write for one key/value, data stores should be stupidly simple to implement.

The folder structure will likely look something like this:

/src
    /actions
    /admin
    /cola
    /reducers
    /store
    /trebek

Trebek will be dramatically simplified, probably getting rid of decorators, which I've come to realize aren't great for this, and will let us upgrade to Babel 6. On top of that, it won't be handling sending messages likely. That'll probably move into the Action Creators, and the responsibility of Trebek will be to normalize input from bots and call the store's dispatch (likely installed as this.dispatch) with the correct actions.

Action Creators are used to define the raw actions, as well as collections of actions. Thunk middleware is included so that we can have action creators that dispatch multiple actions. The action creators will also handle verification of the data to ensure that a dispatch should be happening. This keeps our reducers purely state transitions, and isolates all of the side-effecty stuff in action creators.

Reducers will handle the entire state of the app. My goal is that we can recover from any crash by just recovering the serialized store state.

I'm not 100% on messages right now. I'm thinking that they'll be an action creator that is dispatched as part of another action creator. This will allow trebek to generically call actions which will then consistently pipe messages into slack. The downside of this is that we're using redux concepts (the messages action creator) to just perform a slack update without it being store-backed. But I think this is probably okay. I still need to figure out how Cola fits into this.

The request locking will be moving out of the filesystem and into a promise chain controlled by promise-semaphore. This should make restarts much more graceful, and also won't rely on file-system watchers.

Implications

With more things being held directly in-memory, there's a likelihood that the app's memory usage increases. For now, we're just going to accept it because the benefits outweigh the downsides of this.

At some point in the future, if memory usage becomes an issue, it'll be possible to persist parts of the store in sync data stores to prevent keeping it in memory.

Because everything is in memory and there are no mongodb writes that block responses, there should a performance boost. The one thing that'll still be async is the image generation in cola. This is the last thing that needs to be figured out. We could potentially move Cola to be sync, because then we can handle responses entirely sync, but might hurt performance.

Things Not Figured Out Yet

kesne commented 8 years ago

So after looking into Cola, it looks relatively simple to generate the image buffers sync without changing any code. However the upload code is now the blocker.

Maybe sending messages itself is a promise semaphore that you can pass metadata to and it handles image generation and upload outside of the concern of the app?

kesne commented 8 years ago

For messages, we could also handle it via middleware, so it's a piece of the payload that an action sends. It could return a promise for when sending is complete so that you can .then it to do other things.

kesne commented 8 years ago

After some discussions it looks like this is the direction we'll try to go:

import { fetchStats } from '../../actions/user';

export const features = ['stats'];
export const noLock = true;
export const triggers = [/stats?(?: ([A-Z0-9.\-_]+))?/];

const stats = (dispatch, { contestant, channel }) => dispatch(fetchStats(contestant, channel));

export default stats;
kesne commented 6 years ago

I'm currently rewriting this bot, and this migration will happen then.