bitflicker64 commented on code in PR #535:
URL: https://github.com/apache/hugegraph-doc/pull/535#discussion_r4299379054


##########
content/en/docs/guides/hugegraph-docker-cluster.md:
##########
@@ -195,9 +195,12 @@ The following tables describe current master entrypoints 
and Compose files, not
 | `HG_STORE_GRPC_PORT` | No | `8500` | `grpc.port` |
 | `HG_STORE_REST_PORT` | No | `8520` | `server.port` |
 | `HG_STORE_DATA_PATH` | No | `/hugegraph-store/storage` | `app.data-path` |
+| `HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE` | No | None | 
`rocksdb.total_memory_size`, in bytes |
 
 > **Deprecated aliases**: `PD_ADDRESS` → `HG_STORE_PD_ADDRESS`, `GRPC_HOST` → 
 > `HG_STORE_GRPC_HOST`, `RAFT_ADDRESS` → `HG_STORE_RAFT_ADDRESS`.
 
+RocksDB caches live outside the Java heap. Leave 
`HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE` unset to size them from the container 
memory limit, or set it to a plain number of bytes; see 
`rocksdb.total_memory_size` in the [HStore configuration 
reference](/docs/quickstart/hugegraph/hugegraph-hstore/).

Review Comment:
   Important: "Leave `HG_STORE_ROCKSDB_TOTAL_MEMORY_SIZE` unset to size them 
from the container memory limit" is only true when the container has a limit, 
and none of the Compose files this guide runs set one.
   
   At apache/hugegraph#3355 head (535c2d01b), 
`RocksdbMemoryBudget.cgroupMemoryLimit()` returns -1 when `memory.max` is `max` 
(no limit), and `effectivePhysical()` then uses 
`OperatingSystemMXBean.getTotalMemorySize()`, which is the memory of the whole 
Docker host or Docker Desktop VM. 
`docker/docker-compose-3pd-3store-3server.yml` and 
`docker/docker-compose-hstore.yml` have no `mem_limit` or 
`deploy.resources.limits`. So in the three-node setup this guide describes, 
with the 12 GB Docker Desktop allocation it recommends on line 15, each of the 
three Stores separately sizes its budget from about 12 GiB: with the launcher's 
2 GiB heap and 1 GiB counted for direct memory that is about (12 - 2 - 1 - 1.5) 
/ 2 = 3.75 GiB each, about 11 GiB of off-heap caches for the three Stores in a 
VM that also runs three PDs, three Servers and Hubble. A reader of this 
sentence would expect the budget to follow a per-container limit that does not 
exist.
   
   Please say what happens without a limit, for example: "Leave it unset to 
size them from the container memory limit, or from the memory of the Docker 
host (the Docker Desktop VM) when the container has no limit. The Compose files 
above set no limit, so each Store sizes its budget from the whole host; set a 
memory limit on the Store services or pin this variable when several Stores 
share one host." Make the same change to line 202 of 
`content/cn/docs/guides/hugegraph-docker-cluster.md`, and add "when one is set; 
otherwise the host's memory" to "In a container the starting point is the 
cgroup memory limit" in the `rocksdb.total_memory_size` row of both 
`hugegraph-hstore.md` pages.



##########
content/en/docs/quickstart/hugegraph/hugegraph-hstore.md:
##########
@@ -205,8 +206,8 @@ This file contains only `rocksdb` and Actuator exposure 
settings.
 
 | Key | Master distribution value | Code default | Meaning |
 |-----|---------|--------------|---------|
-| `rocksdb.total_memory_size` | `32000000000` | `51539607552` | Memory budget 
shared by all RocksDB instances on this node. When absent or `0`, the node uses 
the JVM max heap instead. |
-| `rocksdb.write_buffer_size` | `32000000` | `33554432` | Memtable size in 
bytes. When absent or `0`, the node uses `total_memory_size / 1000`. |
+| `rocksdb.total_memory_size` | `0` | Auto-sized, see Meaning | Memory budget 
in bytes shared by all RocksDB instances on this node, split into the write 
cache and the block cache by `write_buffer_ratio`. Both caches are off-heap, so 
`-Xmx` does not limit them, and a fixed 1 GiB Raft log cache comes on top. When 
absent or `0`, the node uses half of the memory left after the JVM max heap, 
`MaxDirectMemorySize` (counted as the heap size, at most 1 GiB, when unset) and 
a 1.5 GiB reserve, clamped to 256 MiB to 32 GB. In a container the starting 
point is the cgroup memory limit, including a limit set only on a parent cgroup 
the Store can see. An `application-pd.yml` kept from an earlier release still 
sets `32000000000`; set it to `0` to auto-size. A positive value is used as 
given and must be plain bytes, at least `1048576` (1 MiB); a smaller value or 
one such as `4GB` stops startup. |

Review Comment:
   Minor: line 150 (unchanged) still says "The following tables were checked 
against distribution values and Java defaults at Server master commit 
`2f827d6e8c9c62ae858f2fc122b3a192d015e2f4`". At that commit both 
`application-pd.yml` files set `total_memory_size: 32000000000` and 
`AppConfig.init()` falls back to `Runtime.maxMemory()` and computes the 
disruptor size without the minimum of 1, so this row, the `write_buffer_size` 
row and the `raft.disruptorBufferSize` row no longer match the commit the page 
says it was checked against. apache/hugegraph#3355's description already notes 
that this commit should move to its merge commit once both PRs land.
   
   Please update the commit on line 150 here and on line 149 of 
`content/cn/docs/quickstart/hugegraph/hugegraph-hstore.md` to the #3355 merge 
commit before this PR merges, or until then add a note that the RocksDB and 
disruptor rows describe apache/hugegraph#3355.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to