feat: node host: add IsNodeInitialized

[nolotz]

Hi there,

I’m facing similar issues as in #266 and think this might be a reliable way to check for shard readiness.

Some functions like NAReadLocalNode will throw a panic if your shard isn’t ready and I would like to bypass those.

Thanks

[nolotz]

I noticed that the API is generally built more towards synchronous calls and tried to follow that, but I would prefer to expose the node’s initializedC channel in this case.

What’s your view on this, @lni?

[lni]

Thanks for the PR.

When the replica is not ready, you will normally get a ErrShardNotReady error.

You got those panics because you didn't call ReadIndex() first. If you read the godoc of the ReadLocalNode() method, it says that -

ReadLocalNode is only allowed to be called after receiving a RequestCompleted notification from the ReadIndex method.

A successfully completed call to ReadIndex ensures that your node is ready. Given that NAReadLocalNode is just a no allocate variant of ReadLocalNode, its godoc should clearly state that NAReadLocalNode has the exact same requirement quoted above.

If you just want to read the replica and you don't care about linearizability, then you should use the StaleRead method.

[lni]

I noticed that the API is generally built more towards synchronous calls and tried to follow that, but I would prefer to expose the node’s initializedC channel in this case.

The API is actually built towards async variants of the calls, the synchronous version is built on top the async variant as a convenient wrapper.

If possible, I'd like to keep that channel private so users can have one less thing/API to worry about. I have the feeling that it already has way too many public methods exposed.

That being said, I do agree that it will be useful to provide a convenient way so users don't have to check the return value of their calls to handle such replica not ready error. I am thinking maybe just add an option so when StartReplica is called, it only returns when the replica is ready to be used. How do you like this approach?

[nolotz]

Thanks for the PR.

When the replica is not ready, you will normally get a ErrShardNotReady error.

You got those panics because you didn't call ReadIndex() first. If you read the godoc of the ReadLocalNode() method, it says that -

ReadLocalNode is only allowed to be called after receiving a RequestCompleted notification from the ReadIndex method.

A successfully completed call to ReadIndex ensures that your node is ready. Given that NAReadLocalNode is just a no allocate variant of ReadLocalNode, its godoc should clearly state that NAReadLocalNode has the exact same requirement quoted above.

If you just want to read the replica and you don't care about linearizability, then you should use the StaleRead method.

Thanks for the explanation, @lni! I might have misused the RequestState in that case and will be better off with StaleRead.

Those bytes slices looked just too tempting.. :unamused:

[nolotz]

I noticed that the API is generally built more towards synchronous calls and tried to follow that, but I would prefer to expose the node’s initializedC channel in this case.

The API is actually built towards async variants of the calls, the synchronous version is built on top the async variant as a convenient wrapper.

If possible, I'd like to keep that channel private so users can have one less thing/API to worry about. I have the feeling that it already has way too many public methods exposed.

That being said, I do agree that it will be useful to provide a convenient way so users don't have to check the return value of their calls to handle such replica not ready error. I am thinking maybe just add an option so when StartReplica is called, it only returns when the replica is ready to be used. How do you like this approach?

I could use this feature, so if you don’t mind, I will create a follow-up merge request for that.