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.

Sign up for GitHub

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

Jump to bottom

ISPN-8740 Refactor backup-write commands (triangle) #5725

Merged
merged 1 commit into from Feb 2, 2018
Merged

ISPN-8740 Refactor backup-write commands (triangle) #5725

merged 1 commit into from Feb 2, 2018

Conversation

pruivo
Copy link
Member

@pruivo pruivo commented Feb 2, 2018

@pruivo pruivo requested a review from rvansa February 2, 2018 11:22
rvansa
rvansa approved these changes Feb 2, 2018
View reviewed changes
Copy link
Member

@rvansa rvansa left a comment

Choose a reason for hiding this comment

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

Very boring PR.

core/src/main/java/org/infinispan/commands/triangle/BackupWriteCommand.java Outdated

@Override
public void writeTo(ObjectOutput output) throws IOException {
CommandInvocationId.writeTo(output, commandInvocationId);
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't define writeTo/readFrom in abstract commands, to force these methods in actual classes. Rename to protected final writeBase?

core/src/main/java/org/infinispan/commands/triangle/BackupWriteCommand.java Outdated

abstract WriteCommand createWriteCommand();

abstract Object anyKey();
Copy link
Member

Choose a reason for hiding this comment

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

This method looks weird. I wonder if we should put the segmentId to constructor (and send it over the wire), or at least define the getSegmentId() separately for each command... Take a second thought about this, please.

@danberindei
Copy link
Member

@pruivo would you like this to go into 9.2.0.Final?

@pruivo
Copy link
Member Author

pruivo commented Feb 2, 2018

@rvansa updated:

  • set the segment-id in the primary. removed the KeyPartitioner from the backup commands
  • removed deprecated method usage: CacheEntry.setValid()
  • removed writeTo()/readFrom() from the abstract commands.

@danberindei yep :)

@rvansa rvansa merged commit c9904db into infinispan:master Feb 2, 2018
@pruivo pruivo deleted the t_8740_refactor_bck_cmd branch July 5, 2022 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvansa rvansa rvansa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@pruivo @danberindei @rvansa