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

[Doc] Cluster deployment #746

Merged
merged 5 commits into from Aug 12, 2019

Conversation

@darionyaphet
Copy link
Contributor

commented Aug 9, 2019

In this PR, I append a document to description cluster deployment and config arguments.

Show resolved Hide resolved docs/deploy-cluster.md Outdated
Show resolved Hide resolved docs/deploy-cluster.md Outdated
Show resolved Hide resolved docs/deploy-cluster.md Outdated
@whitewum

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

some suggestions

@darionyaphet darionyaphet force-pushed the darionyaphet:deploy-cluster branch from b6180e0 to b07acac Aug 9, 2019

@sherman-the-tank
Copy link
Member

left a comment

Thanks for writing up the doc. Well done! 👍

Something not related to this doc itself caught my attention. I think we need to review all flags to make them not only consistent, but also more meaningful

`pid_file` | "pids/nebula-metad.pid" | File to hold the process id.
`daemonize` | true | Whether run as a daemon process.
`cluster_id` | 0 | A unique id for each cluster.
`putTryNum` | 10 | Try num of store clusterId to kvstore.

This comment has been minimized.

Copy link
@sherman-the-tank

sherman-the-tank Aug 9, 2019

Member

This description is somehow confusing...

This comment has been minimized.

Copy link
@darionyaphet

darionyaphet Aug 11, 2019

Author Contributor

I lost attention on it.

Number of attempts to generate cluster ID would be better ?

This comment has been minimized.

Copy link
@sherman-the-tank

sherman-the-tank Aug 11, 2019

Member

Yes, much more understandable 👍

Show resolved Hide resolved docs/deploy-cluster.md Outdated
Show resolved Hide resolved docs/deploy-cluster.md Outdated
`rocksdb_column_family_options` | "" | ColumnFamilyOptions, each option will be given as <option_name>:<option_value> separated by.
`rocksdb_block_based_table_options` | "" | BlockBasedTableOptions, each option will be given as <option_name>:<option_value> separated by.
`batch_reserved_bytes` | 4 * 1024 | Default reserved bytes for one batch operation
`block_cache` | 4 | BlockBasedTable:block_cache : MB

This comment has been minimized.

Copy link
@sherman-the-tank

sherman-the-tank Aug 9, 2019

Member

I think this is also a rocksdb config, right?

It would be great to make all rocksdb configuration start with rocksdb_

`max_outstanding_requests` | 1024 | The max number of outstanding appendLog requests.
`raft_rpc_timeout_ms` | 500 | RPC timeout for raft client.
`accept_log_append_during_pulling` | false | Whether to accept new logs during pulling the snapshot.
`heartbeat_interval` | 5 | Seconds between each heartbeat.

This comment has been minimized.

Copy link
@sherman-the-tank

sherman-the-tank Aug 9, 2019

Member

heartbeat_interval_secs? So that consistent with other interval configs

This comment has been minimized.

Copy link
@darionyaphet

darionyaphet Aug 11, 2019

Author Contributor

Good Point. 👍

I have update it as raft_ heartbeat_interval_secs because it's duplicate with meta client's flag.

@nebula-community-bot

This comment has been minimized.

Copy link

commented Aug 11, 2019

Unit testing failed.

@darionyaphet darionyaphet force-pushed the darionyaphet:deploy-cluster branch from f88195c to f0832f0 Aug 11, 2019

@nebula-community-bot

This comment has been minimized.

Copy link

commented Aug 11, 2019

Unit testing failed.

@darionyaphet darionyaphet force-pushed the darionyaphet:deploy-cluster branch from f0832f0 to 800b264 Aug 11, 2019

@nebula-community-bot

This comment has been minimized.

Copy link

commented Aug 11, 2019

Unit testing passed.

@darionyaphet darionyaphet force-pushed the darionyaphet:deploy-cluster branch from 800b264 to 0e56718 Aug 12, 2019

@nebula-community-bot

This comment has been minimized.

Copy link

commented Aug 12, 2019

Unit testing passed.

@laura-ding
Copy link
Contributor

left a comment

Well done

@nebula-community-bot

This comment has been minimized.

Copy link

commented Aug 12, 2019

Unit testing passed.

@whitewum
Copy link
Contributor

left a comment

I prefer to add some examples.
But the current pr is OK.


First at all, you can download rpm or deb from [Here](https://github.com/vesoft-inc/nebula/releases).

Currently, we have offered `CentOS 7.5`, `CentOS 6.5`, `Ubuntu 1604` and `Ubuntu 1804`'s installation package.

This comment has been minimized.

Copy link
@whitewum

whitewum Aug 12, 2019

Contributor

BTW, I prefer not to use we (use be xxed, or nebula)

@nebula-community-bot

This comment has been minimized.

Copy link

commented Aug 12, 2019

Unit testing passed.

@dutor dutor merged commit 587d7ea into vesoft-inc:master Aug 12, 2019

1 check passed

UnitTest All tests passed.
Details

@darionyaphet darionyaphet deleted the darionyaphet:deploy-cluster branch Aug 12, 2019

@darionyaphet darionyaphet referenced this pull request Aug 13, 2019

Closed

installation #567

Show resolved Hide resolved docs/deploy-cluster.md
Show resolved Hide resolved docs/deploy-cluster.md
Show resolved Hide resolved docs/deploy-cluster.md
Show resolved Hide resolved docs/deploy-cluster.md
Show resolved Hide resolved docs/deploy-cluster.md
Show resolved Hide resolved docs/deploy-cluster.md
`raft_heartbeat_interval_secs` | 5 | Seconds between each heartbeat.
`max_batch_size` | 256 | The max number of logs in a batch.

**Graph Service** support the following config properties.

This comment has been minimized.

Copy link
@Amber1990Zhang

Amber1990Zhang Aug 13, 2019

Contributor

supports

Show resolved Hide resolved docs/deploy-cluster.md
Show resolved Hide resolved docs/deploy-cluster.md
Show resolved Hide resolved src/kvstore/raftex/test/LogAppendTest.cpp
@darionyaphet

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

Thanks for you review :) I will open another PR to fix them. @Amber1990Zhang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.