Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix WAIT_MORE_DATA during stream #35

Merged
merged 5 commits into from
Dec 7, 2023

Conversation

moticless
Copy link
Collaborator

@moticless moticless commented Dec 6, 2023

Required to flush cache in the iterations of the loop that read global PEL as well.

Refactored and emphesis this operation with a distinct function:

/* Some of the element states are iterative in nature. For example, the parsing of
 * a list element is done by iterating over all its items, reading the next element
 * and calling to corresponding handler. The iteration usually starts by reading
 * from the RDB source and then enter into safe-state, process the data and usually
 * callback to corresponding handler. In that context, while the parser takes
 * care to flush the cache on transition to a new state such that the cache will
 * reflect only current state, instead of forcing each iteration to go back to the
 * parserMainLoop() and then return all the way back to the same element-state
 * for the next iteration, we can optimize the iterative states and make short
 * loop that will flush the cache internally at the end of each iteration, by
 * calling the following function and won't go back till the last iteration.
 *
 * By flushing the cache on each iteration, we ensure that if the state iteration
 * gets  interrupted in the middle, the parser can later resume the same interrupted
 * iteration of element state with a relevant cache that reflects only the
 * interrupted iteration. */
static inline RdbStatus updateElementStateIterative(RdbParser *p) {
    bulkPoolFlush(p);
    return RDB_STATUS_OK;
}

Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can't say i fully understand this. let's have a quick voice call so you can explain it to me.

src/lib/parser.c Outdated Show resolved Hide resolved
@oranagra
Copy link
Member

oranagra commented Dec 7, 2023

please update the top comment (for commit comment).
p.s. you moved some functions, which causes difficulties reviewing the diff and disrupts the blame log.. maybe it can be avoided.

@moticless moticless merged commit 22d5276 into redis:main Dec 7, 2023
2 checks passed
@moticless moticless deleted the fix-stream-wait-more-data branch December 7, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants