You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "Denovo1998 (via GitHub)" <gi...@apache.org> on 2025/12/27 12:21:06 UTC
[PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Denovo1998 opened a new pull request, #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645
### Motivation
When running BookKeeper with `DbLedgerStorage`, RocksDB index placement (`indexDirectories`) can have a significant impact on IO behavior and tail latency. The current Helm chart does not provide first-class support for configuring `indexDirectories` or mounting a dedicated volume for it, so index and ledger data often end up sharing the same storage unless users manually customize templates, increasing operational overhead and limiting performance tuning.
### Modifications
* Added a `pulsar.bookkeeper.indexDirectories` helper that supports both `string` and `string[]`, and defaults to `bookkeeper.volumes.index.mountPath` when an index volume is enabled and no explicit value is provided.
* Injected `indexDirectories` into the rendered BookKeeper configuration when applicable (also setting `PULSAR_PREFIX_indexDirectories`).
* Updated the BookKeeper StatefulSet startup logic to `mkdir -p` for all configured `indexDirectories` paths to ensure they exist at runtime.
* Introduced an optional `bookkeeper.volumes.index` volume for a dedicated index mount, including PVC/volumeClaimTemplates support and StorageClass generation when needed.
* Updated `values.yaml` with documentation and examples for `bookkeeper.volumes.index` and `bookkeeper.indexDirectories`.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#discussion_r2661397827
##########
charts/pulsar/templates/bookkeeper-configmap.yaml:
##########
@@ -55,6 +55,12 @@ data:
{{- else }}
ledgerDirectories: "/pulsar/data/bookkeeper/ledgers"
{{- end }}
+ {{- $indexDirs := include "pulsar.bookkeeper.indexDirectories" . }}
+ {{- if $indexDirs }}
+ # Configure RocksDB/index location for DbLedgerStorage. When unset, BookKeeper falls back to ledgerDirectories.
+ indexDirectories: {{ $indexDirs | quote }}
+ PULSAR_PREFIX_indexDirectories: {{ $indexDirs | quote }}
Review Comment:
It seems that `PULSAR_PREFIX_indexDirectories` is unnecessary.
The `apply-config-from-env.py` script takes commented out lines into account. Tested this in apache/pulsar working dir:
```shell
indexDirectories=foo docker/pulsar/scripts/apply-config-from-env.py conf/bookkeeper.conf
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#discussion_r2660708888
##########
charts/pulsar/values.yaml:
##########
@@ -818,6 +818,19 @@ bookkeeper:
size: 10Gi
# storageClassName: existent-storage-class
mountPath: /pulsar/data/bookkeeper/ledgers1
+ # Optional dedicated volume for BookKeeper indexes / RocksDB (`indexDirectories`).
+ # Useful when ledgers are on HDD but you want indexes on NVMe.
+ index:
+ enabled: true
+ name: index
+ size: 10Gi
+ local_storage: true
+ storageClassName: existent-storage-class
+ # storageClass:
+ # ...
+ # selector:
+ # ...
+ mountPath: /pulsar/data/bookkeeper/index
Review Comment:
All tests in CI break since this is enabled by default. Please disable this by default.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "Denovo1998 (via GitHub)" <gi...@apache.org>.
Denovo1998 commented on code in PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#discussion_r2661383153
##########
charts/pulsar/values.yaml:
##########
@@ -818,6 +818,19 @@ bookkeeper:
size: 10Gi
# storageClassName: existent-storage-class
mountPath: /pulsar/data/bookkeeper/ledgers1
+ # Optional dedicated volume for BookKeeper indexes / RocksDB (`indexDirectories`).
+ # Useful when ledgers are on HDD but you want indexes on NVMe.
+ index:
+ enabled: false
+ name: index
+ size: 10Gi
+ local_storage: true
+ storageClassName: existent-storage-class
Review Comment:
Okay, that's right.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "Denovo1998 (via GitHub)" <gi...@apache.org>.
Denovo1998 commented on code in PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#discussion_r2661436447
##########
charts/pulsar/templates/bookkeeper-configmap.yaml:
##########
@@ -55,6 +55,12 @@ data:
{{- else }}
ledgerDirectories: "/pulsar/data/bookkeeper/ledgers"
{{- end }}
+ {{- $indexDirs := include "pulsar.bookkeeper.indexDirectories" . }}
+ {{- if $indexDirs }}
+ # Configure RocksDB/index location for DbLedgerStorage. When unset, BookKeeper falls back to ledgerDirectories.
+ indexDirectories: {{ $indexDirs | quote }}
+ PULSAR_PREFIX_indexDirectories: {{ $indexDirs | quote }}
Review Comment:
Okay.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "Denovo1998 (via GitHub)" <gi...@apache.org>.
Denovo1998 commented on PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#issuecomment-3693943596
@lhotari PTAL.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "Denovo1998 (via GitHub)" <gi...@apache.org>.
Denovo1998 commented on code in PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#discussion_r2661186970
##########
charts/pulsar/values.yaml:
##########
@@ -818,6 +818,19 @@ bookkeeper:
size: 10Gi
# storageClassName: existent-storage-class
mountPath: /pulsar/data/bookkeeper/ledgers1
+ # Optional dedicated volume for BookKeeper indexes / RocksDB (`indexDirectories`).
+ # Useful when ledgers are on HDD but you want indexes on NVMe.
+ index:
+ enabled: true
+ name: index
+ size: 10Gi
+ local_storage: true
+ storageClassName: existent-storage-class
+ # storageClass:
+ # ...
+ # selector:
+ # ...
+ mountPath: /pulsar/data/bookkeeper/index
Review Comment:
Okay, Already modified.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "Denovo1998 (via GitHub)" <gi...@apache.org>.
Denovo1998 commented on PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#issuecomment-3710294100
> LGTM, thanks for the contribution @Denovo1998
It's alright, just a small favor.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#discussion_r2661243984
##########
charts/pulsar/values.yaml:
##########
@@ -818,6 +818,19 @@ bookkeeper:
size: 10Gi
# storageClassName: existent-storage-class
mountPath: /pulsar/data/bookkeeper/ledgers1
+ # Optional dedicated volume for BookKeeper indexes / RocksDB (`indexDirectories`).
+ # Useful when ledgers are on HDD but you want indexes on NVMe.
+ index:
+ enabled: false
+ name: index
+ size: 10Gi
+ local_storage: true
+ storageClassName: existent-storage-class
Review Comment:
well, it looks like storage class will default to `local-storage` if `storageClassName` isn't set and `local_storage` is set to `true`. Since the storageClassName is commented out for ledger volumes, it's more consistent to comment it out here too.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for BookKeeper indexDirectories configuration [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #645:
URL: https://github.com/apache/pulsar-helm-chart/pull/645#discussion_r2661230959
##########
charts/pulsar/values.yaml:
##########
@@ -818,6 +818,19 @@ bookkeeper:
size: 10Gi
# storageClassName: existent-storage-class
mountPath: /pulsar/data/bookkeeper/ledgers1
+ # Optional dedicated volume for BookKeeper indexes / RocksDB (`indexDirectories`).
+ # Useful when ledgers are on HDD but you want indexes on NVMe.
+ index:
+ enabled: false
+ name: index
+ size: 10Gi
+ local_storage: true
+ storageClassName: existent-storage-class
Review Comment:
I think that `storageClassName` should be commented out by default too. In that case, I'd assume that the default storage class would get used, similar to how the ledgers storageClassName is configured.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org