[
https://issues.apache.org/jira/browse/SOLR-16924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17845105#comment-17845105
]
Jason Gerlowski commented on SOLR-16924:
----------------------------------------
bq. got very confused for a while until I realized we have two RestoreCore
classes [...] Wow; ok! The first implements the V2 API and calls the second,
the V1 API.
Not exactly. 'admin.api.RestoreCore' is our V2 API. But 'handler.RestoreCore'
isn't the v1 API - it's an implementation class that contains a lot of the
logic to do the restore itself. It's used by both the v1 and v2 API endpoints.
The actual v1 entrypoint, such as it is, is CoreAdminHandler (which calls
RestoreCoreOp).
So, to summarize the logic flow for both v1 and v2:
*v2*
{code}
admin.api.RestoreCore.restoreCore (v2 entrypoint)
|-> handler.RestoreCore.doRestore (actual implementation logic)
{code}
*v1*
{code}
CoreAdminHandler.handleRequestBody (v1 entrypoint)
|-> RestoreCoreOp.execute
|---> admin.api.RestoreCore.restoreCore
|-----> handler.RestoreCore.doRestore (actual implementation logic)
{code}
As you pointed out, the v1 code path calls the v2 codepath (which then calls
the business logic). Which is surprising/unusual, but it's something we did
very intentionally: it ensures that v1 and v2 endpoints for an API remain in
sync (i.e. it prevents developers from adding a param to v1 without updating
v2). And by having v1 consume the v2 code, it sets us up to transition away
from v1 at some point in the future without needing to modify the v2 endpoint
or feature logic. (Funnily enough, this is actually an idea I got from you!
I'll add the link in here if I can dig it up...)
----
Moving on to your actual questions:
bq. Are there any problems or bugs here?
Not that I can see.
There might be some maintenance-cost improvements we can make: move
'handler.RestoreCore' to a different package or rename it to make it clearer
that it's not "api" code maybe?
bq. Like, should the change in this PR be placed at the end of V1 instead?
If by "at the end of v1" you mean the "handler.RestoreCore" business-logic
class - then probably?
For functionality like restore-core that has a dedicated business-logic class,
I'd expect 'admin.api.RestoreCore' to be mostly "api" logic (e.g. validating
inputs) and for all logic pertaining to restoring to go into
"handler.RestoreCore". That's not a bug, to be clear - but it's a nice
division-of-responsibilities to try and maintain.
bq. Should the RestoreCore classes be merged?
I don't think so, nope. Two reasons:
1. It's valuable to keep "api" concerns and business logic separate.
2. The business logic in handler.RestoreCore is reused by Solr for several
other restore-like operations as well (e.g. "install-shard"). We wouldn't want
to tie it to any particular API since we have 2 or 3 different APIs that call
into this business logic.
Hopefully that helps clarify; lmk if I missed any questions or it still doesn't
make sense?
> Restore: Have RESTORECORE set the UpdateLog state
> --------------------------------------------------
>
> Key: SOLR-16924
> URL: https://issues.apache.org/jira/browse/SOLR-16924
> Project: Solr
> Issue Type: Improvement
> Reporter: David Smiley
> Priority: Minor
> Fix For: 9.5
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> This is a refactoring improvement designed to simplify & clarify a step in
> collection restores. One of the final phases of RestoreCmd (collection
> restore) is to call REQUESTAPPLYUPDATES on each newly restored replica in
> order to transition the state of the UpdateLog to ACTIVE (not actually to
> apply updates). The underlying call on the UpdateLog could instead be done
> inside RESTORECORE at the end with explanatory comments as to the intent. I
> think this makes more sense that RESTORECORE finish with its UpdateLog ready.
> And it's strange/curious to see requests in the cluster to apply updates
> from an updateLog when there is none to do! Adding clarifying comments is
> important.
> See my comment:
> https://issues.apache.org/jira/browse/SOLR-12065?focusedCommentId=17751792&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17751792
> I think there isn't any back-compat concern.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
