HDDS-11418. leader executor framework #7406
Conversation
| public OMResponse submitInternal(OMRequest omRequest, String clientId, long callId) throws ServiceException { | ||
| requestInProgress.incrementAndGet(); | ||
| RequestContext requestContext = new RequestContext(); | ||
| requestContext.setRequest(omRequest); |
| (r, th) -> handleAfterExecution(requestContext, th)); | ||
| OmLockOpr.OmLockInfo lockInfo = null; | ||
| try { | ||
| OMRequestBase requestBase = OmRequestFactory.createClientRequest(omRequest, om); |
There was a problem hiding this comment.
Reusing Request name seemed wrong in the original code itself. Request == a bunch of attributes (nouns) and a verb for what needs to be done. These ClientRequests embed the entire logic for executing the code. They in essense capture the lamba to be performed for each request.
| OMRequestBase requestBase = OmRequestFactory.createClientRequest(omRequest, om); | |
| OMRequestBase requestBase = OmRequestFactory.createRequestExecutor(omRequest, om); |
There was a problem hiding this comment.
or createRequestProcessors
| requestContext.setUuidClientId(clientId); | ||
| requestContext.setCallId(callId); | ||
| requestContext.setRequestBase(requestBase); | ||
| requestContext.getRequestBase().authorize(om); |
There was a problem hiding this comment.
checkACLs should be fine, it will make it very clear what is expected to take place.
| requestContext.setRequestBase(requestBase); | ||
| requestContext.getRequestBase().authorize(om); | ||
| lockInfo = requestContext.getRequestBase().lock(omLockOpr); | ||
| validate(requestContext.getRequest()); |
There was a problem hiding this comment.
It's ok to break up validate into independent methods which validate only one thing
- ValidateParams
- ValidatePrepareState
- ...
There will be a long list of validations that occur in submitInternal and that is fine.
| if (om.isRatisEnabled()) { | ||
| om.getOmRatisServer().getOmStateMachine().getPostExecutor().registerIndexNotifier(this::indexNotifier); | ||
| } else { | ||
| uniqueIndex.set(PostExecutor.initLeaderIndex(om)); |
There was a problem hiding this comment.
Cleaner to initialize this first.
What changes were proposed in this pull request?
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11418
How was this patch tested?