Merge pull request #632 from tulios/shuffle-seed-brokers

Randomize order of seed brokers
tulios · Jan 14, 2020 · dc10e77 · dc10e77
2 parents 0b70749 + b76a5ef
commit dc10e77
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 9 deletions.
diff --git a/src/cluster/__tests__/connectionBuilder.spec.js b/src/cluster/__tests__/connectionBuilder.spec.js
@@ -28,11 +28,11 @@ describe('Cluster > ConnectionBuilder', () => {
     })
   })
 
-  test('creates a new connection using the first the seed broker', () => {
+  test('creates a new connection using a random broker', () => {
     const connection = builder.build()
     expect(connection).toBeInstanceOf(Connection)
-    expect(connection.host).toEqual('host.test')
-    expect(connection.port).toEqual(7777)
+    expect(connection.host).toBeOneOf(['host.test', 'host2.test', 'host3.test'])
+    expect(connection.port).toBeOneOf([7777, 7778, 7779])
     expect(connection.ssl).toEqual(ssl)
     expect(connection.sasl).toEqual(sasl)
     expect(connection.clientId).toEqual(clientId)
@@ -43,11 +43,13 @@ describe('Cluster > ConnectionBuilder', () => {
   })
 
   test('when called without host and port iterates throught the seed brokers', () => {
-    expect(builder.build()).toEqual(expect.objectContaining({ host: 'host.test', port: 7777 }))
-    expect(builder.build()).toEqual(expect.objectContaining({ host: 'host2.test', port: 7778 }))
-    expect(builder.build()).toEqual(expect.objectContaining({ host: 'host3.test', port: 7779 }))
-    expect(builder.build()).toEqual(expect.objectContaining({ host: 'host.test', port: 7777 }))
-    expect(builder.build()).toEqual(expect.objectContaining({ host: 'host2.test', port: 7778 }))
+    const connections = Array(brokers.length)
+      .fill()
+      .map(() => {
+        const { host, port } = builder.build()
+        return `${host}:${port}`
+      })
+    expect(connections).toIncludeSameMembers(brokers)
   })
 
   test('accepts overrides for host, port and rack', () => {

diff --git a/src/cluster/connectionBuilder.js b/src/cluster/connectionBuilder.js
@@ -1,5 +1,6 @@
 const Connection = require('../network/connection')
 const { KafkaJSNonRetriableError } = require('../errors')
+const shuffle = require('../utils/shuffle')
 
 const validateBrokers = brokers => {
   if (!brokers || brokers.length === 0) {
@@ -23,14 +24,15 @@ module.exports = ({
 }) => {
   validateBrokers(brokers)
 
+  const shuffledBrokers = shuffle(brokers)
   const size = brokers.length
   let index = 0
 
   return {
     build: ({ host, port, rack } = {}) => {
       if (!host) {
         // Always rotate the seed broker
-        const [seedHost, seedPort] = brokers[index++ % size].split(':')
+        const [seedHost, seedPort] = shuffledBrokers[index++ % size].split(':')
         host = seedHost
         port = Number(seedPort)
       }