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

[FIXED] Randomize discovered server URLs if allowed #319

Merged
merged 2 commits into from
May 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 9 additions & 6 deletions src/srvpool.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,19 @@ _addURLToPool(natsSrvPool *pool, char *sURL, bool implicit, const char *tlsName)
}

static void
_shufflePool(natsSrvPool *pool)
_shufflePool(natsSrvPool *pool, int offset)
{
int i, j;
natsSrv *tmp;

if (pool->size <= 1)
if (pool->size <= offset+1)
return;

srand((unsigned int) nats_NowInNanoSeconds());

for (i = 0; i < pool->size; i++)
for (i = offset; i < pool->size; i++)
{
j = rand() % (i + 1);
j = offset + rand() % (i + 1 - offset);
tmp = pool->srvrs[i];
pool->srvrs[i] = pool->srvrs[j];
pool->srvrs[j] = tmp;
Expand Down Expand Up @@ -321,6 +321,8 @@ natsSrvPool_addNewURLs(natsSrvPool *pool, const natsUrl *curUrl, char **urls, in
}
s = _addURLToPool(pool, url, true, tlsName);
}
if ((s == NATS_OK) && added && pool->randomize)
_shufflePool(pool, 1);
}

natsStrHash_Destroy(tmp);
Expand Down Expand Up @@ -360,6 +362,7 @@ natsSrvPool_Create(natsSrvPool **newPool, natsOptions *opts)
// Set the current capacity. The array of urls may have to grow in
// the future.
pool->cap = poolSize;
pool->randomize = !opts->noRandomize;

// Map that helps find out if an URL is already known.
s = natsStrHash_Create(&(pool->urls), poolSize);
Expand All @@ -371,8 +374,8 @@ natsSrvPool_Create(natsSrvPool **newPool, natsOptions *opts)
if (s == NATS_OK)
{
// Randomize if allowed to
if (!(opts->noRandomize))
_shufflePool(pool);
if (pool->randomize)
_shufflePool(pool, 0);
}

// Normally, if this one is set, Options.Servers should not be,
Expand Down
1 change: 1 addition & 0 deletions src/srvpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ typedef struct __natsSrvPool
natsStrHash *urls;
int size;
int cap;
bool randomize;

} natsSrvPool;

Expand Down
53 changes: 29 additions & 24 deletions test/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -5579,18 +5579,30 @@ _checkPool(natsConnection *nc, char **expectedURLs, int expectedURLsCount)
}

static natsStatus
checkPoolOrderDidNotChange(natsConnection *nc, char **urlsAfterPoolSetup, int initialPoolSize)
checkNewURLsAddedRandomly(natsConnection *nc, char **urlsAfterPoolSetup, int initialPoolSize)
{
natsStatus s;
int i;
char **currentPool = NULL;
int currentPoolSize = 0;

s = natsConnection_GetServers(nc, &currentPool, &currentPoolSize);
for (i= 0; (s == NATS_OK) && (i < initialPoolSize); i++)
if (s == NATS_OK)
{
if (strcmp(urlsAfterPoolSetup[i], currentPool[i]))
s = NATS_ERR;
// Reset status to error by default. If we find a new URL
// before the end of the initial list, we consider success
// and return.
s = NATS_ERR;
for (i= 0; i < initialPoolSize; i++)
{
// If one of the position in initial list is occupied
// by a new URL, we are ok.
if (strcmp(urlsAfterPoolSetup[i], currentPool[i]))
{
s = NATS_OK;
break;
}
}
}
if (currentPool != NULL)
{
Expand Down Expand Up @@ -5796,18 +5808,14 @@ test_AsyncINFO(void)
natsConnection_Destroy(nc);
nc = NULL;

// Check that pool may be randomized on setup, but new URLs are always
// added at end of pool.
// Check that new URLs are added at random places in the pool
if (s == NATS_OK)
{
int initialPoolSize = 0;
char **urlsAfterPoolSetup = NULL;
const char *newURLs[] = {
"localhost:6222",
"localhost:7222",
"localhost:8222\", \"localhost:9222",
"localhost:10222\", \"localhost:11222\", \"localhost:12222,",
};
const char *newURLs = "\"impA:4222\", \"impB:4222\", \"impC:4222\", "\
"\"impD:4222\", \"impE:4222\", \"impF:4222\", \"impG:4222\", "\
"\"impH:4222\", \"impI:4222\", \"impJ:4222\"";

s = natsOptions_Create(&opts);
if (s == NATS_OK)
Expand All @@ -5824,18 +5832,15 @@ test_AsyncINFO(void)
FAIL("Unable to setup test");

// Add new urls
for (i=0; i<(int)(sizeof(newURLs)/sizeof(char*)); i++)
{
snprintf(buf, sizeof(buf), "INFO {\"connect_urls\":[\"%s\"]}\r\n", newURLs[i]);
PARSER_START_TEST;
s = natsParser_Parse(nc, buf, (int)strlen(buf));
if (s == NATS_OK)
{
// Check that pool order does not change up to the new addition(s).
s = checkPoolOrderDidNotChange(nc, urlsAfterPoolSetup, initialPoolSize);
}
testCond((s == NATS_OK) && (nc->ps->state == OP_START));
}
snprintf(buf, sizeof(buf), "INFO {\"connect_urls\":[%s]}\r\n", newURLs);
test("New URLs are added randomly: ");
s = natsParser_Parse(nc, buf, (int)strlen(buf));
if (s == NATS_OK)
s = checkNewURLsAddedRandomly(nc, urlsAfterPoolSetup, initialPoolSize);
testCond((s == NATS_OK) && (nc->ps->state == OP_START));

test("First URL should not have been changed: ")
testCond((s == NATS_OK) && !strcmp(nc->srvPool->srvrs[0]->url->fullUrl, urlsAfterPoolSetup[0]));

if (urlsAfterPoolSetup != NULL)
{
Expand Down