mhart / kinesalite

An implementation of Amazon's Kinesis built on LevelDB
MIT License
808 stars 86 forks source link

Use with promises a bit awkward #44

Closed cantide5ga closed 7 years ago

cantide5ga commented 7 years ago

Fist off, thanks for https://github.com/mhart/kinesalite and https://github.com/mhart/kinesis - it opens up a lot of opportunities.

My issue is with the callback being outside of the 'setTimeout' in certain operations i.e.:

store.deleteStreamDb(key, function(err) {
        if (err) return cb(err)

        setTimeout(function() {
          metaDb.del(key, function(err) {
            if (err && !/Database is not open/.test(err)) console.error(err.stack || err)
          })
        }, store.deleteStreamMs)

        cb()
      })

I'm forced to explicitly juggle a lower operation state with a higher delay:

kinesaliteServer = kinesalite({path: './build/mydb', deleteStreamMs: 0})
//...
kinesis.deleteStream({ StreamName: 'testStream' })
.promise()
.then(() => { // <-- I would expect the API to return a meaningful resolved promise
     return Promise.delay(10); 
 })
 .then(done);

Even when promises aren't involved, there is a race condition here. Placing the cb() inside the setTimeout makes this more manageable in my case.

Am I using it incorrectly or not understanding the intent?

mhart commented 7 years ago

@cantide5ga I think you're probably not using callbacks correctly (this library doesn't use promises, nor does kinesis)

kinesalite is just intended to be a server – you probably shouldn't be calling kinesalite methods directly – you should just start it up and then treat it as though you're talking with AWS Kinesis (ie, use a client library, like kinesis or the AWS-SDK)

The deleteStreamMs option is in there to emulate the production AWS behaviour – that is, it takes a while in the real system for a stream to delete, so this project simulates that instead of just assuming the stream is deleted immediately. In a real system, you should just wait until the stream is in the state that you want – checking every so often to see what state it's in (but not explicitly using the kinesalite deleteStreamMs value, because that's got nothing to do with production Kinesis)

The AWS-SDK has a method for this, waitFor, as described here: http://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/Kinesis.html#waitFor-property

If you wanted to use the kinesis library, it's a bit more low-level, but you could do something like this:

function waitUntilDeleted(streamName, cb) {
  kinesis.request('DescribeStream', {StreamName: streamName}, function(err, data) {
    if (err && err.name == 'ResourceNotFoundException') return cb() // no stream, success!
    if (err) return cb(err) // some other error occurred
    setTimeout(waitUntilDeleted, 1000, streamName, cb) // stream exists, wait 1 sec and check again
  })
}

And then if you wanted to combine this with the delete itself, then you could do something like:

function deleteStreamAndWait(streamName, cb) {
  kinesis.request('DeleteStream', {StreamName: streamName}, function(err, data) {
    if (err) return cb(err)
    setTimeout(waitUntilDeleted, 1000, streamName, cb) // wait 1 sec and then check
  })
}

And of course if you wanted to promisify this, there are many ways to do it:

function deleteStreamAndWait(streamName) {
  return new Promise((resolve, reject) => {
    kinesis.request('DeleteStream', {StreamName: streamName}, function(err, data) {
      if (err) return reject(err)
      setTimeout(waitUntilDeleted, 1000, streamName, err => err ? reject(err) : resolve())
    })
  })
}
cantide5ga commented 7 years ago

this library doesn't use promises, nor does kinesis

@mhart, unless I'm misunderstanding you, fyi aws sdk's support promises out of the box now.

My example exercises the API they support:

kinesis.deleteStream({ StreamName: 'testStream' })
.promise()
.then(() => { // <-- I would expect the API to return a meaningful resolved promise
     return Promise.delay(10); 
 })
 .then(done);

kinesalite is just intended to be a server – you probably shouldn't be calling kinesalite methods directly – you should just start it up and then treat it as though you're talking with AWS Kinesis (ie, use a client library, like kinesis or the AWS-SDK)

Most surely am not doing this. I touched your code to work in the expectation and test it out - current workaround is using kinesalite exactly as you suggest and am using the aws-sdk as I do with the actual Kinesis service.

Thanks for your reply however, I'll chew on this.

mhart commented 7 years ago

@cantide5ga AWS' SDK does support promises – I was referring to the https://github.com/mhart/kinesis client (which I thought was what you were using in your examples – my apologies)