Skip to content
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.

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

WIP: v4 #642

Merged
merged 15 commits into from
Jul 1, 2018
Merged

WIP: v4 #642

merged 15 commits into from
Jul 1, 2018

Conversation

luin
Copy link
Collaborator

@luin luin commented Jun 23, 2018

In this major version, we would drop the support for node 4 (#408) in order to use the built-in Promise instead of bluebird (#639 ).

Other plans are:

  • Support Per-Request retry limitations.
  • Upgrade redis-parser to v3
  • Easier way to iterate the key space via SCAN and ES6 Iterators.
  • Use TypeScript to rewrite this module for cleaner code and official type decroations

Cluster related:

Any other ideas? @shaharmor @Salakar @AVVS

shaharmor and others added 6 commits June 18, 2018 18:10
…ch back.

BREAKING CHANGE:
This change makes all the code that rely on the features provided by Bluebird not working
anymore. For example, `redis.get('foo').timeout(500)` now should be failed since the native
Promise doesn't support the `timeout` method.

You can switch back to the Bluebird implementation by setting `Redis.Promise`:

```
const Redis = require('ioredis')
Redis.Promise = require('bluebird')

const redis = new Redis()

// Use bluebird
assert.equal(redis.get().constructor, require('bluebird'))

// You can change the Promise implementation at any time:
Redis.Promise = global.Promise
assert.equal(redis.get().constructor, global.Promise)
```
BREAKING CHANGE:
The `new` keyword is required explicitly. So `Redis(/* options */)`
will not work, use `new Redis(/* options */)` instead.
@shaharmor
Copy link
Collaborator

Cluster:

  • Refresh slots upon a disconnection from one of the cluster nodes
  • Restructure the cluster initialization code so that the initial slots refresh call will be done only after connecting to the first node. This will help with issues like Cluster: Failed to refresh slots cache. when options.redisOptions.enableOfflineQueue = false #581 where the user is setting enableOfflineQueue: false which causes the slots refresh call to fail immediately and disconnect from the node.
  • Consider setting enableOfflineQueue: true explicitly on the internal cluster nodes - @luin correct me if I'm wrong, but the offline queue of the internal node shouldn't be used in cluster mode because when ever the socket will disconnect the node will automatically be removed.

Other:

  • Remove callback support? Not sure why we have to support callbacks if we support promises. If anyone really needs callback support, they can "callbackify" the promises themselves. Why do we need the entire library to support it?

@luin
Copy link
Collaborator Author

luin commented Jun 24, 2018

Refresh slots upon a disconnection from one of the cluster nodes

A disconnection won't lead the update of slots immediatially. A cluster will consider a master to be failing only after NODE_TIMEOUT has elapsed.

So most of time we only need refetch the slots in two different situations (https://redis.io/topics/cluster-spec#clients-first-connection-and-handling-of-redirections):

  • At startup in order to populate the initial slots configuration.
  • When a MOVED redirection is received.

However, given a failing of a slave won't trigger a MOVED error, so refresh slots intervally help us solve the problem.

Remove callback support? Not sure why we have to support callbacks if we support promises. If anyone really needs callback support, they can "callbackify" the promises themselves. Why do we need the entire library to support it?

Good idea. I've seriously considered removing callback support. However, what makes me tend to keep callback support in this major version is that many built-in functions (fs.readFile) still follow callback-style, and util.promisify was absent until v8.0.0. The major frameworks (e.g. express, mocha) continue to be friendly to the callback style, so there should be many legacy codes prevent developers from migrating to the ioredis v4 if we drop callback support at this moment.

We may do this on the next major version, which only supports node >= v8 that support async functions.

luin and others added 2 commits June 25, 2018 01:53
…er command

#634, #61

BREAKING CHANGE:
The maxRetriesPerRequest is set to 20 instead of null (same behavior as ioredis v3)
by default. So when a redis server is down, pending commands won't wait forever
until the connection become alive, instead, they only wait about 10s (depends on the
retryStrategy option)
BREAKING CHANGE:
`Cluster#connect()` will be resolved when the connection
status become `ready` instead of `connect`.
luin and others added 5 commits June 30, 2018 11:29
BREAKING CHANGE:
`Redis#connect()` will be resolved when status is ready
instead of `connect`:

```
const redis = new Redis({ lazyConnect: true })
redis.connect().then(() => {
  assert(redis.status === 'ready')
})
```
Offline queue should be enabled on the internal
cluster nodes so that we don't need to wait for the
`ready` event before sending commands to the node.
@luin luin merged commit 622975d into master Jul 1, 2018
@luin luin deleted the v4 branch July 1, 2018 16:53
@luin
Copy link
Collaborator Author

luin commented Jul 1, 2018

Just release a beta version v4.0.0-0. The remaining issues will be resolved in the next beta version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants