Skip to content

Resolves #117: Support feeding in the datacenter_id option from start up command to …#134

Merged
apkar merged 4 commits intoFoundationDB:masterfrom
dongxinEric:feature/specify-dcid-when-start-up
Apr 1, 2019
Merged

Resolves #117: Support feeding in the datacenter_id option from start up command to …#134
apkar merged 4 commits intoFoundationDB:masterfrom
dongxinEric:feature/specify-dcid-when-start-up

Conversation

@dongxinEric
Copy link
Copy Markdown
Contributor

…the FDB Database created.

@dongxinEric dongxinEric self-assigned this Mar 28, 2019
@dongxinEric dongxinEric requested review from apkar and brownleej March 28, 2019 22:17
@dongxinEric
Copy link
Copy Markdown
Contributor Author

dongxinEric commented Mar 28, 2019

@brownleej expected syntax is the same for an optional argument:

--fdb_datacenter_id=SOME_AWESOME_ID

@dongxinEric
Copy link
Copy Markdown
Contributor Author

#117

Comment thread src/DocLayer.actor.cpp Outdated
Comment thread src/DocLayer.actor.cpp Outdated
@dongxinEric dongxinEric requested a review from apkar March 28, 2019 23:04
@apkar apkar requested review from ajbeamon and removed request for brownleej March 28, 2019 23:08
@dongxinEric
Copy link
Copy Markdown
Contributor Author

@ajbeamon can you take a look at this? I briefly checked where the datacenter_id got used in the fdbclient code and it seems like OK to allow user to pass in random string, which in worse scenario will only make FDB thinks this client is not in any DC that it resides in and fall back to use latency.

Copy link
Copy Markdown
Contributor

@ajbeamon ajbeamon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this looks fine. I made a minor tweak to the wording of the option documentation that you can take if you want, but otherwise I approve.

Comment thread src/DocLayer.actor.cpp Outdated
Co-Authored-By: dongxinEric <jiangzian1987dx@gmail.com>
Copy link
Copy Markdown
Contributor

@apkar apkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@apkar apkar merged commit c4cb6a3 into FoundationDB:master Apr 1, 2019
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.

3 participants