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

[#678] Revert to current tip should be a non operation #684

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

yersan
Copy link
Contributor

@yersan yersan commented May 31, 2024

if (marker.getOperation() != operation) {
if (ProsperoLogger.ROOT_LOGGER.isDebugEnabled()) {
ProsperoLogger.ROOT_LOGGER.debugf("The candidate server has been prepared for different operation [%s].", marker.getOperation().getText());
if (operation == Type.REVERT && metadata.getRevisions().get(0).getName().equals(hash)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@yersan the hash in this case is a record of the state of the updated/reverted server at the point of executing the prepare operation not the state of the candidate after operation is performed

Copy link
Contributor Author

@yersan yersan Jun 4, 2024

Choose a reason for hiding this comment

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

@spyrkob Got it, I see it. I thought the hash of the marker was going to give me the tip of the candidate server. In any case, even if I were able to get the hash of the candidate server at this point, it won't be useful at all to compare with the revision of the current one since the hash depends on other factors and not only about whether it points out to the same server state.

Is there any utility that can be used at this point to compare if there are changes by using the filesystem (comparing both directories content)? and, would this point and approach be valid to achieve it? I mean, by using the verifyCandidate method to compare and verify whether the candidate is valid, WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@yersan I think you can use https://github.com/wildfly-extras/prospero/blob/main/prospero-common/src/main/java/org/wildfly/prospero/actions/ApplyCandidateAction.java#L342 to get the file changes between candidate and origin server.

What's the use case where the hash compare wouldn't work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @spyrkob, I'll have a closer look. I still don't understand how this method can provide the differences between two installations, as far as I can see it just takes a directory as a source and then uses Galleon to compute the differences, but differences regarding that? It only uses the installation dir, so I'm not sure how it can be used to compare and provide differences between two directories (installation dir and candidate dir). Anyway, I'll have a look.

What's the use case where the hash compare wouldn't work?

My naive approach was to check whether the tip of the installation dir was the same as the tip of the candidate dir. I thought that the hash returned by the marker file would give me the hash of the candidate dir. So, I wanted to compare both, and if they were the same, I could say that the candidate dir was not going to bring in any changes to the installation dir, and therefore return with an error.

However, I think hash values are also dependent on the timestamp about when they were created, so the above approach comparing those hashes seems it is not going to work at all. I guess, I would need to compare filesystem changes at this point to discover whether there are differences between installation dir and candidate dir.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@yersan you're right, findChanges() will find user changes in one installation, not a diff between two. Maybe we can use Galleon's .hashes directories in both servers and compare them to get a diff of server files? Would need to compare the configuration files in .installation folder separately though.

As for hash - another option might be to add the target hash of the revert operation to the CandidateProperties and read it from there.

Copy link
Collaborator

@spyrkob spyrkob left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @yersan!

@spyrkob spyrkob merged commit be9242a into wildfly-extras:main Jun 17, 2024
7 checks passed
@yersan yersan deleted the issue-678 branch July 8, 2024 16:56
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.

Revert to current tip should be a non operation
2 participants