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

MODE-1279 - Fixed repository blocking after performing a full-text search multiple times #299

Closed
wants to merge 2 commits into from

Conversation

hchiorean
Copy link
Member

The fix was part of the #207 pull request, which basically removed explicit synchronization around the LuceneSearchProcessor and Engine.

In addition:

  • the Lucene version was updated from 3.1.0 to 3.4.0
  • the LuceneSearchSession was updated to avoid the possible loss of content when indexing the same content in the same workspace from multiple threads

…arch multiple times

The fix was part of the ModeShape#207 pull request, which basically removed explicit synchronization around the LuceneSearchProcessor and Engine.

In addition:
- the lucene version was updated from 3.1.0 to 3.4.0
- the LuceneSearchSession was updated to avoid the possible loss of content when indexing the same content in the same workspace from multiple threads
@@ -71,10 +71,11 @@
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you intend to make these changes to this file, which appear at first glance to be just formatting changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changes came as part of the original pull request patch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking again at the file, they aren't only formatting changes: the queries have an ORDER BY clause added to them. Do you want to discard those ?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, we don't want to discard those. I now remember adding the ORDER BY.

@rhauch
Copy link
Contributor

rhauch commented Feb 14, 2012

Overall, the changes look very good except for the files that seem to have only formatting changes. If that's the case, can we remove those files from the pull-request?

@hchiorean
Copy link
Member Author

Do you want me to revert the above mentioned files ?

@rhauch
Copy link
Contributor

rhauch commented Feb 14, 2012

If you wouldn't mind, try reverting the two files, moving those 'request.add(NoMoreFederatedRequests)' lines, and running the tests again.

@hchiorean
Copy link
Member Author

Ok, will update the mentioned files are per comment(s)

this.session = this.engine.getRepository(repositoryName)
.login(new SimpleCredentials("superuser",
"superuser".toCharArray()));
this.session = this.engine.getRepository(repositoryName).login();
Copy link
Member Author

Choose a reason for hiding this comment

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

This change was done because there isn't any authentication configured for the "superuser"/"superuser" credentials and therefore I was getting an "access denied exception" from the repo (or smth similar)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I didn't catch that. Any chance of limiting the changes to just the affected lines?

@hchiorean
Copy link
Member Author

Updated code as per comments: reverted styling and moved NoMoreFederatedRequest inside finally block.

@rhauch
Copy link
Contributor

rhauch commented Feb 14, 2012

Merged into the 'master' branch.

@rhauch rhauch closed this Feb 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants