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 request processor endless loop #90

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

JackyWoo
Copy link
Contributor

@JackyWoo JackyWoo commented Oct 9, 2023

Which issues of this PR fixes:

This PR try to fix #89

Change log:

  1. Add KeeperCommon.h which contains common prototypes such as RequestForSession
  2. Rename KeeperCommon.h to KeeperUtils.h
  3. Add RequestId instead of UInt28(session_id, xid)
  4. Fix request processor endless loop
  5. sessionCleanerTask will not finishSession
  6. Some code refactoring.

@JackyWoo JackyWoo added the bug Something isn't working label Oct 9, 2023
@JackyWoo JackyWoo added this to the Release v2.0.2 milestone Oct 9, 2023
@JackyWoo JackyWoo requested a review from nicelulu October 9, 2023 13:33
@JackyWoo JackyWoo force-pushed the request_processor_endless_loop branch from eee45f5 to 8e6114d Compare October 9, 2023 15:49
@JackyWoo JackyWoo marked this pull request as draft October 9, 2023 15:55
@JackyWoo JackyWoo force-pushed the request_processor_endless_loop branch 2 times, most recently from d5b7677 to 8ebc875 Compare October 10, 2023 12:51
@JackyWoo JackyWoo force-pushed the request_processor_endless_loop branch from 8ebc875 to 7f403e1 Compare October 10, 2023 13:35
@JackyWoo JackyWoo marked this pull request as ready for review October 10, 2023 13:35
@JackyWoo
Copy link
Contributor Author

The CI error is data race and will be fixed here #90

processCommittedRequest(committed_request_size);

/// 3. process error requests
processErrorRequest();
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to get the number of current errors in advance, before moving to the pending queue, so as to ensure that the processed errors have been added to the pending queue.

Copy link
Contributor Author

@JackyWoo JackyWoo Oct 16, 2023

Choose a reason for hiding this comment

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

Now errors is a map, we can't handle n items in order, we can use list instead of map to hold errors.

if (found_error)
LOG_WARNING(log, "Found error request, We should terminate the processing of committed(write) requests.");
else
process_not_in_pending_queue();
Copy link
Contributor

Choose a reason for hiding this comment

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

May previous request is error but not in errors queue yet, may we need traverse pending queue to judgement commit request whether in pending queue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here first_pending_request.request->xid != committed_request.request->xid, maybe there is no need to traverse pending queue.

@JackyWoo JackyWoo requested a review from nicelulu October 17, 2023 01:53
@nicelulu nicelulu merged commit 58c3ff9 into JDRaftKeeper:master Oct 20, 2023
6 checks passed
JackyWoo added a commit that referenced this pull request Jan 5, 2024
* Fix request processor endless loop

* Fix typo

* Use list for errors
JackyWoo added a commit that referenced this pull request Jan 8, 2024
* Fix request processor endless loop

* Fix typo

* Use list for errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request Processor endless loop.
2 participants