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

[controller][admin-tool][vpj][test] Add a multi-region config to controllers and cleanup many configs #1076

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

nisargthakkar
Copy link
Contributor

@nisargthakkar nisargthakkar commented Jul 23, 2024

Add a multi.region config to controllers and cleanup many other configs

  1. Use multi.region config to control certain features instead of controlling them via explicit configs.
    • Native-replication - Enabled in multi-region mode. Disabled in single-region mode.
    • Admin channel consumption - Enabled in multi-region mode. Disabled in single-region mode.
      • This is still allowed to be disabled via LiveConfig for store migration purposes.
    • Controller allowing BATCH push via API - Disabled in multi-region mode. Enabled in single-region mode.
    • Active-active replication enabled on a controller - Enabled in multi-region mode. Disabled in single-region mode.
  2. Commands and code to enable and disable Native-replication for a cluster has been removed as that is the only mode.
  3. The following configs are now obsolete, and removed:
    • enable.native.replication.for.batch.only
    • enable.native.replication.for.hybrid
    • enable.native.replication.as.default.for.batch.only
    • enable.native.replication.as.default.for.hybrid
    • enable.active.active.replication.as.default.for.batch.only.store
    • admin.topic.remote.consumption.enabled
    • child.controller.admin.topic.consumption.enabled
      • Only removed from controller config
      • It is still used in LiveConfigs during store migration
    • active.active.enabled.on.controller
    • controller.enable.batch.push.from.admin.in.child
  4. The following configs were unused and now removed:
    • kafka.min.log.compaction.lag.ms
    • topic.cleanup.send.concurrent.delete.requests.enabled
    • topic.deletion.status.poll.interval.ms
    • topic.creation.throttling.time.window.ms
    • topic.manager.kafka.operation.timeout.ms
    • admin.consumption.timeout.minute
    • amplification.factor
    • server.ingestion.isolation.metric.request.timeout.seconds

How was this PR tested?

GH CI

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to explain your proposed changes and call out the behavior change.

@nisargthakkar nisargthakkar force-pushed the multiRegionBackendConfig branch 6 times, most recently from 6f6dbeb to 495f03f Compare July 23, 2024 21:50
…rollers and infer many configs

1. Use multi.region config to control certain features instead of controlling them via explicit configs.
    * Native-replication - Enabled in multi-region mode. Disabled in single-region mode.
    * Admin channel consumption - Enabled in multi-region mode. Disabled in single-region mode.
        * This is still allowed to be disabled via LiveConfig for store migration purposes.
    * Controller allowing BATCH push via API - Disabled in multi-region mode. Enabled in single-region mode.
    * Active-active replication enabled on a controller - Enabled in multi-region mode. Disabled in single-region mode.
2. Commands and code to enable and disable Native-replication for a cluster has been removed as that is the only mode.
3. The following configs are now obsolete, and removed:
    * enable.native.replication.for.batch.only
    * enable.native.replication.for.batch.only
    * enable.native.replication.for.hybrid
    * enable.native.replication.as.default.for.batch.only
    * enable.native.replication.as.default.for.hybrid
    * enable.active.active.replication.as.default.for.batch.only.store
    * enable.active.active.replication.as.default.for.batch.only.store
    * admin.topic.remote.consumption.enabled
    * child.controller.admin.topic.consumption.enabled
        * Only removed from controller config
        * It is still used in LiveConfigs during store migration
    * active.active.enabled.on.controller
    * controller.enable.batch.push.from.admin.in.child
4. The following configs were unused and now removed:
    * kafka.min.log.compaction.lag.ms
    * topic.cleanup.send.concurrent.delete.requests.enabled
    * topic.deletion.status.poll.interval.ms
    * topic.creation.throttling.time.window.ms
    * topic.manager.kafka.operation.timeout.ms
    * admin.consumption.timeout.minute
    * amplification.factor
    * server.ingestion.isolation.metric.request.timeout.seconds
@nisargthakkar nisargthakkar marked this pull request as ready for review July 24, 2024 01:11
@nisargthakkar nisargthakkar changed the title [WIP][controller][admin-tool][vpj][test] Add a multi-region config to controllers and infer many configs [WIP][controller][admin-tool][vpj][test] Add a multi-region config to controllers and cleanup many configs Jul 24, 2024
@nisargthakkar nisargthakkar changed the title [WIP][controller][admin-tool][vpj][test] Add a multi-region config to controllers and cleanup many configs [controller][admin-tool][vpj][test] Add a multi-region config to controllers and cleanup many configs Jul 24, 2024
@nisargthakkar nisargthakkar force-pushed the multiRegionBackendConfig branch 2 times, most recently from ed70e58 to 37d6763 Compare July 25, 2024 14:06
@mynameborat
Copy link
Contributor

Looks good to me. If you can remove the unused constructor that would be good.

@nisargthakkar
Copy link
Contributor Author

Thanks a lot @mynameborat

@nisargthakkar nisargthakkar merged commit df1e78b into linkedin:main Jul 25, 2024
32 checks passed
@nisargthakkar nisargthakkar deleted the multiRegionBackendConfig branch July 25, 2024 17:34
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.

2 participants