On 2021/11/10 20:19, Bharath Rupireddy wrote:
Thanks for the patch. It looks good to me other than the following comment:
Thanks for the review!
1) Can't we determine the wait event type based on commandName in ExecuteRecoveryCommand instead of passing it as an extra param?
Yes, that's possible. But isn't it uglier to make ExecuteRecoveryCommand() have the map of command name and wait event? So I feel inclined to avoid adding something like the following code into the function... Thought? if (strcmp(commandName, "recovery_end_command") == 0) wait_event_info = WAIT_EVENT_RECOVERY_END_COMMAND; else if (strcmp(commandName, "archive_command_command") == 0) ... Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION