Unable to set redpanda.remote.write and redpanda.remote.read together in a single rpk command

[patrickangeles]

Version & Environment

v21.11.15

For some reason, this command:

rpk topic alter-config thelog -s redpanda.remote.read=true -s redpanda.remote.write=true

only sets remote write to true:

rpk topic describe thelog      
SUMMARY
=======
NAME        thelog
PARTITIONS  1
REPLICAS    1

CONFIGS
=======
KEY                     VALUE                          SOURCE
cleanup.policy          delete                         DYNAMIC_TOPIC_CONFIG
compression.type        producer                       DEFAULT_CONFIG
message.timestamp.type  CreateTime                     DEFAULT_CONFIG
partition_count         1                              DYNAMIC_TOPIC_CONFIG
redpanda.datapolicy     function_name:  script_name:   DEFAULT_CONFIG
redpanda.remote.read    false                          DYNAMIC_TOPIC_CONFIG
redpanda.remote.write   true                           DYNAMIC_TOPIC_CONFIG
replication_factor      1                              DYNAMIC_TOPIC_CONFIG
retention.bytes         100000                         DYNAMIC_TOPIC_CONFIG
retention.ms            604800000                      DEFAULT_CONFIG
segment.bytes           10000                          DYNAMIC_TOPIC_CONFIG

Setting each parameter in separate rpk commands works.
Note this does not happen for other topic configuration parameters. For example the following works as expected.

rpk topic alter-config thelog  -s retention.ms=12345 -s segment.bytes=123

JIRA Link: CORE-899

[twmb]

This sounds like some config parsing bug within redpanda itself, cc @piyushredpanda

[LenaAn]

Can confirm that it looks like redpanda issue, cause setting via kcl claims the same result

> kcl admin configs alter Lenka --inc --kv set:redpanda.remote.read=true set:redpanda.remote.write=true

> kcl admin configs describe Lenka                                                                      
cleanup.policy          delete                         DEFAULT_CONFIG
compression.type        producer                       DEFAULT_CONFIG
message.timestamp.type  CreateTime                     DEFAULT_CONFIG
partition_count         1                              DYNAMIC_TOPIC_CONFIG
redpanda.datapolicy     function_name:  script_name:   DEFAULT_CONFIG
redpanda.remote.read    true                           DYNAMIC_TOPIC_CONFIG
redpanda.remote.write   false                          DYNAMIC_TOPIC_CONFIG
replication_factor      1                              DYNAMIC_TOPIC_CONFIG
retention.bytes         -1                             DEFAULT_CONFIG
retention.ms            604800000                      DEFAULT_CONFIG
segment.bytes           1073741824                     DEFAULT_CONFIG
[16:28:52] [cost 0.036s] kcl admin configs describe Lenka

Although it doesn't reproduce in alter_config_test.cc

[LenaAn]

Also setting them both to false doesn't work:

[23:48:45] [~/work/lenaan/redpanda] git(dev) 🔥 ❱❱❱ rpk topic describe Lenka          
...
redpanda.remote.read    true                           DYNAMIC_TOPIC_CONFIG
redpanda.remote.write   true                           DYNAMIC_TOPIC_CONFIG
...

[23:48:48] [~/work/lenaan/redpanda] git(dev) 🔥 ❱❱❱ rpk topic alter-config Lenka -s redpanda.remote.read=false  redpanda.remote.write=false
TOPIC                        STATUS
Lenka                        OK
redpanda.remote.write=false  UNKNOWN_TOPIC_OR_PARTITION

[23:48:56] [~/work/lenaan/redpanda] git(dev) 🔥 ❱❱❱ rpk topic describe Lenka                                                               
SUMMARY
=======
NAME        Lenka
PARTITIONS  1
REPLICAS    1

CONFIGS
=======
KEY                     VALUE                          SOURCE
...
redpanda.remote.read    false                          DYNAMIC_TOPIC_CONFIG
redpanda.remote.write   true                           DYNAMIC_TOPIC_CONFIG
...

[LenaAn]

The root cause is that we represent shadow_indexing_mode as one flag in incremental_topic_updates, and we can't represent changes to remote.read and remote.write at the same time. For example, add_shadow_indexing_flag(drop_fetch, archival) = drop_fetch and we loose the information that we should also set archival to true