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


Reply via email to