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

ISPN-7990 Provide a way to remove an entry from all indexes #5279

Merged
merged 2 commits into from Jul 27, 2017

Conversation

gustavocoding
Copy link

@gustavocoding
Copy link
Author

@karesti Once you have the test to reproduce the issue of lack of values in the context during topology change, you can test against this branch.

@rvansa
Copy link
Member

rvansa commented Jul 26, 2017

@gustavonalle Needs rebase.

@rvansa
Copy link
Member

rvansa commented Jul 27, 2017

LGTM, @anistor could you bless this?

@anistor
Copy link
Member

anistor commented Jul 27, 2017

Sure, I'll have a look after the meeting.

@gustavocoding
Copy link
Author

Rebased

* Remove entries from all indexes by key
*/
void removeFromIndexes(TransactionContext transactionContext, Object key) {
transactionContext = transactionContext == null ? makeTransactionalEventContext() : transactionContext;
Copy link
Member

Choose a reason for hiding this comment

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

Assigning same value to itself is a bit strange. Why not do it like if (transactionContext == null) { transactionContext = makeTransactionalEventContext(); } ?

Copy link
Author

Choose a reason for hiding this comment

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

This used everywhere in the QI

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, and it looks a bit silly. Let's leave like that in the name of tradition :)

@@ -31,4 +31,7 @@
* @return collection of Work instances
*/
Collection<Work> createPerEntityWorks(Object entity, Serializable id, WorkType workType);

Collection<Work> createPerEntityIdWorks(Serializable id, IndexedTypeIdentifier entityType, WorkType workType);
Copy link
Member

Choose a reason for hiding this comment

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

A bit of javadoc would be nice.

Copy link
Author

Choose a reason for hiding this comment

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

ok. Will change a bit this method, it does not need to return a collection

@@ -9,6 +9,8 @@
import java.util.Set;
Copy link
Member

Choose a reason for hiding this comment

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

A bit too much javadoc reformatting.

Copy link
Author

Choose a reason for hiding this comment

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

This came from the "official" formatter unfortunately. Should we avoid in general to reformat comments?

Copy link
Member

Choose a reason for hiding this comment

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

I we don't change any content, then better not reformat them.

@gustavocoding
Copy link
Author

Updated.

@anistor anistor merged commit b1ec2aa into infinispan:master Jul 27, 2017
@anistor
Copy link
Member

anistor commented Jul 27, 2017

Integrated in master. Thanks @gustavonalle !

@gustavocoding gustavocoding deleted the ISPN-7990 branch February 20, 2018 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants