hashgraph / hedera-services

Crypto, token, consensus, file, and smart contract services for the Hedera public ledger
Apache License 2.0
280 stars 124 forks source link

Reconnect: track information about expected data-pieces atomically #12545

Open anthony-swirldslabs opened 5 months ago

anthony-swirldslabs commented 5 months ago

As noted at https://github.com/hashgraph/hedera-services/pull/11724 , the Learner TreeView implementation uses three different queues to track three separate pieces of information about the same expected datagram from the teacher side, like this:

        expectedChildren.add(parent == null ? 0 : getChildPath(parent, childIndex));
        expectedNodeAlreadyPresent.add(nodeAlreadyPresent);
        expectedOriginalExists.add(original != null);

and then it also has a boilerplate code that asserts the sync status of all the three queues, like that:

        assert expectedOriginalExists.isEmpty() == expectedChildren.isEmpty()
                        && expectedChildren.isEmpty() == expectedNodeAlreadyPresent.isEmpty()
                : "All three should match";

And it then has to make sure the queues are always being added to/removed from altogether, at all times.

This approach is error-prone and makes the code difficult to understand and maintain.

The proposal is to model the information about the expected datagram in a single object (e.g. a record) and maintain just a single queue of such objects in the learner TreeView implementation.

anthony-swirldslabs commented 3 months ago

Note: when researching https://github.com/hashgraph/hedera-services/issues/13064 , a case was identified where the queues become empty and hence the learner task thread terminates. However, the counter of expected datagrams is maintained separately in the AsyncInputStream, and it seems to be non-zero which keeps the input read running, or at least this is how it looks.

Regardless, that counter is the 4th place where we track the information about the expected datagrams.

It would be great to unify all the 4 places into one to ensure the absence of any bugs.