Fail CLIENT IMPORT-SOURCE When Import Mode is Disabled and Reflect Client Import Source State in CLIENT LIST

[PingXie]

          I think we should clean it up though. Also, shall we update `CLIENT LIST` output to reflect the import source mode?

Originally posted by @hpatro in #1185 (comment)

[PingXie]

Some random thoughts for consideration

1. should we make import-mode a debug config (DEBUG_CONFIG)? there is a OOM risk for server operating in this mode so if the server gets OOM killed, it would be good to tell whether this mode was left on accidentally from the crash report.

2. along the same OOM risk line, can we consider ref-counting "import-source" clients and automatically turn on/off the server "import-mode" based on the number of such clients? This would eliminate the need of explicit clean up (to clear "import-mode" upon the completion of the import). And then we avoid introducing a new server config.

+@soloestoy

[soloestoy]

1. should we make import-mode a debug config (DEBUG_CONFIG)? there is a OOM risk for server operating in this mode so if the server gets OOM killed, it would be good to tell whether this mode was left on accidentally from the crash report.

Good idea.

2. along the same OOM risk line, can we consider ref-counting "import-source" clients and automatically turn on/off the server "import-mode" based on the number of such clients? This would eliminate the need of explicit clean up (to clear "import-mode" upon the completion of the import). And then we avoid introducing a new server config.

This is indeed an effective method to "automatically" toggle import-mode, and it can save a configuration.

However, there are a few concerns:

1. When the "import-source" is 0, valkey automatically exits import-mode, but we don't know whether the source has truly exited or if there is an exceptional disconnection. If the disconnection is due to network issues, the imported data might be expired and deleted upon reconnection.
2. Enabling the server's status by the client feels quite risky to me. Although I haven't encountered a real-world example yet, intuitively, I think using a configuration would be safer. An explicit configuration would more clearly define its scope and method of use.

[PingXie]

2. Enabling the server's status by the client feels quite risky to me. Although I haven't encountered a real-world example yet, intuitively, I think using a configuration would be safer. An explicit configuration would more clearly define its scope and method of use.

I am thinking more in the context of managed services where a user wants to import data into a managed cluster. My main concern is the (user) client never coming back and turning off the import mode on the server. The server then ends up growing its memory footprint unbounded. Making this command a superuser command wouldn't help much either IMO. So how can a server protects itself from a potentially buggy client?

1. When the "import-source" is 0, valkey automatically exits import-mode, but we don't know whether the source has truly exited or if there is an exceptional disconnection. If the disconnection is due to network issues, the imported data might be expired and deleted upon reconnection.

What if we allow some delay, say 5 mins, before turning off the import mode on the server, after the last "import-source" client drops? This would allow the client to connect back in case of network issues without breaking the "import" semantics while at the same time prevents the memory footprint from growing unbounded on the server?

Just brainstorming ...