PP-783: race condition in mom hook transport initiated by provisioning


#1

Hi All,
I have posted an Interface change documentation for PP-783.
Request you to provide your comments.

Thanks,
Riyaz


#2

Hey Riyaz,
Log messages are usually unstable. There needs to be a reason to stabilize it. Usually it is because there is a consumer of the log message that requires it to get their work done. Who is the consumer of this new log message? To change a stable interface, you need to deprecate it for 1yr. A stable interface means you need to write automated tests for it. There is work involved with a stable interface. If it’s unstable, we can change it if we see a need to without any warning.

I’m not saying don’t make it stable. I’m just asking you to think about it before doing so.

Bhroam


#3

I looked closer at the code and your new log message will only be printed on a malloc() failure. There are many such log messages which are not stable interfaces. I really don’t think this needs to be stable.

Bhroam


#4

Hi @riyazhakki,

I agree with Bhroam that we should not make this interface stable.

Below are other comments -
“If not, it sets a timely task to recheck again” should be --> If not, it creates a timed task to check again.

We can change some wording of the log message -
“Unable to set task for is_hooks_sync_done, requeing the job.”

Thanks,
Prakash


#5

It looks good with two changes: Please use a semicolon instead of the
comma, and please use “requeueing” or “requeuing” instead of “requeing”.
Thanks.


#6

Hi Bhroam,
Thanks for reviewing the document.
Yes, log message will only be printed on a malloc() failure and there is no consumer of this log.
I have updated the Interface document.


#7

Hi @prakashcv13,
Thanks for reviewing the document.
I have addressed your comment.


#8

Hi @agurban,
Thanks for reviewing the document.
I have addressed the comment.


#9

Thank you @riyazhakki,

The document looks good to me. I sign-off.

-Prakash


#10

Our new change control guidelines say that unstable interfaces do not need to be included in external design documents. This is such a case. It’s up to you. You can leave it in, or delete it.

I’d say to not proliferate our project design space with small EDDs. I’d delete the EDD. That’s just my opinion.

The EDD doesn’t need my sign off, but I’ll provide it anyway :slight_smile:
Bhroam