You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/08/19 18:35:30 UTC
[GitHub] [lucene-solr] s1monw opened a new pull request #1764: Ensure we only rollback IW once
s1monw opened a new pull request #1764:
URL: https://github.com/apache/lucene-solr/pull/1764
Today we might rollback IW more than once if we hit an exception during
the rollback code when we shutdown. This change moves the rollback code outside
of the try block to ensure we always roll back but never roll back twice.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msokolov commented on a change in pull request #1764: Ensure we only rollback IW once
Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #1764:
URL: https://github.com/apache/lucene-solr/pull/1764#discussion_r474285099
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
##########
@@ -2023,4 +2023,44 @@ public void eval(MockDirectoryWrapper dir) throws IOException {
dir.close();
}
+
+
+ public void testOnlyRollbackOnceOnException() throws IOException {
+ AtomicBoolean once = new AtomicBoolean(false);
+ InfoStream stream = new InfoStream() {
+ @Override
+ public void message(String component, String message) {
+ if ("TP".equals(component) && "rollback before checkpoint".equals(message)) {
+ if (once.compareAndSet(false, true)) {
+ throw new RuntimeException("boom");
+ } else {
+ throw new AssertionError("has been rolled back twice");
+ }
+
+ }
+ }
+
+ @Override
+ public boolean isEnabled(String component) {
+ return "TP".equals(component);
+ }
+
+ @Override
+ public void close() {
+ }
+ };
+ try (Directory dir = newDirectory()) {
+ try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig().setInfoStream(stream)){
+ @Override
+ protected boolean isEnableTestPoints() {
+ return true;
+ }
+ }) {
+ }
+ } catch (RuntimeException x) {
Review comment:
I'm really happy there were no votes for runtimeException (IDE generated code fancult vote)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1764: Ensure we only rollback IW once
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1764:
URL: https://github.com/apache/lucene-solr/pull/1764#discussion_r473654082
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
##########
@@ -2023,4 +2023,44 @@ public void eval(MockDirectoryWrapper dir) throws IOException {
dir.close();
}
+
+
+ public void testOnlyRollbackOnceOnException() throws IOException {
+ AtomicBoolean once = new AtomicBoolean(false);
+ InfoStream stream = new InfoStream() {
+ @Override
+ public void message(String component, String message) {
+ if ("TP".equals(component) && "rollback before checkpoint".equals(message)) {
+ if (once.compareAndSet(false, true)) {
+ throw new RuntimeException("boom");
+ } else {
+ throw new AssertionError("has been rolled back twice");
+ }
+
+ }
+ }
+
+ @Override
+ public boolean isEnabled(String component) {
+ return "TP".equals(component);
+ }
+
+ @Override
+ public void close() {
+ }
+ };
+ try (Directory dir = newDirectory()) {
+ try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig().setInfoStream(stream)){
+ @Override
+ protected boolean isEnableTestPoints() {
+ return true;
+ }
+ }) {
+ }
+ } catch (RuntimeException x) {
Review comment:
yeah why x.... :D
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw merged pull request #1764: Ensure we only rollback IW once
Posted by GitBox <gi...@apache.org>.
s1monw merged pull request #1764:
URL: https://github.com/apache/lucene-solr/pull/1764
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1764: Ensure we only rollback IW once
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1764:
URL: https://github.com/apache/lucene-solr/pull/1764#discussion_r473681215
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
##########
@@ -2023,4 +2023,44 @@ public void eval(MockDirectoryWrapper dir) throws IOException {
dir.close();
}
+
+
+ public void testOnlyRollbackOnceOnException() throws IOException {
+ AtomicBoolean once = new AtomicBoolean(false);
+ InfoStream stream = new InfoStream() {
+ @Override
+ public void message(String component, String message) {
+ if ("TP".equals(component) && "rollback before checkpoint".equals(message)) {
+ if (once.compareAndSet(false, true)) {
+ throw new RuntimeException("boom");
+ } else {
+ throw new AssertionError("has been rolled back twice");
+ }
+
+ }
+ }
+
+ @Override
+ public boolean isEnabled(String component) {
+ return "TP".equals(component);
+ }
+
+ @Override
+ public void close() {
+ }
+ };
+ try (Directory dir = newDirectory()) {
+ try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig().setInfoStream(stream)){
+ @Override
+ protected boolean isEnableTestPoints() {
+ return true;
+ }
+ }) {
+ }
+ } catch (RuntimeException x) {
Review comment:
let the bikeshedding begin 👯 I call a vote...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1764: Ensure we only rollback IW once
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1764:
URL: https://github.com/apache/lucene-solr/pull/1764#discussion_r473683479
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
##########
@@ -2023,4 +2023,44 @@ public void eval(MockDirectoryWrapper dir) throws IOException {
dir.close();
}
+
+
+ public void testOnlyRollbackOnceOnException() throws IOException {
+ AtomicBoolean once = new AtomicBoolean(false);
+ InfoStream stream = new InfoStream() {
+ @Override
+ public void message(String component, String message) {
+ if ("TP".equals(component) && "rollback before checkpoint".equals(message)) {
+ if (once.compareAndSet(false, true)) {
+ throw new RuntimeException("boom");
+ } else {
+ throw new AssertionError("has been rolled back twice");
+ }
+
+ }
+ }
+
+ @Override
+ public boolean isEnabled(String component) {
+ return "TP".equals(component);
+ }
+
+ @Override
+ public void close() {
+ }
+ };
+ try (Directory dir = newDirectory()) {
+ try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig().setInfoStream(stream)){
+ @Override
+ protected boolean isEnableTestPoints() {
+ return true;
+ }
+ }) {
+ }
+ } catch (RuntimeException x) {
Review comment:
In the spirit peaceful atmosphere: let's call it e + x = "ex"...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1764: Ensure we only rollback IW once
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1764:
URL: https://github.com/apache/lucene-solr/pull/1764#discussion_r473675798
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
##########
@@ -2023,4 +2023,44 @@ public void eval(MockDirectoryWrapper dir) throws IOException {
dir.close();
}
+
+
+ public void testOnlyRollbackOnceOnException() throws IOException {
+ AtomicBoolean once = new AtomicBoolean(false);
+ InfoStream stream = new InfoStream() {
+ @Override
+ public void message(String component, String message) {
+ if ("TP".equals(component) && "rollback before checkpoint".equals(message)) {
+ if (once.compareAndSet(false, true)) {
+ throw new RuntimeException("boom");
+ } else {
+ throw new AssertionError("has been rolled back twice");
+ }
+
+ }
+ }
+
+ @Override
+ public boolean isEnabled(String component) {
+ return "TP".equals(component);
+ }
+
+ @Override
+ public void close() {
+ }
+ };
+ try (Directory dir = newDirectory()) {
+ try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig().setInfoStream(stream)){
+ @Override
+ protected boolean isEnableTestPoints() {
+ return true;
+ }
+ }) {
+ }
+ } catch (RuntimeException x) {
Review comment:
I think I have to veto this. I liked x better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dnhatn commented on a change in pull request #1764: Ensure we only rollback IW once
Posted by GitBox <gi...@apache.org>.
dnhatn commented on a change in pull request #1764:
URL: https://github.com/apache/lucene-solr/pull/1764#discussion_r473254890
##########
File path: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
##########
@@ -2023,4 +2023,44 @@ public void eval(MockDirectoryWrapper dir) throws IOException {
dir.close();
}
+
+
+ public void testOnlyRollbackOnceOnException() throws IOException {
+ AtomicBoolean once = new AtomicBoolean(false);
+ InfoStream stream = new InfoStream() {
+ @Override
+ public void message(String component, String message) {
+ if ("TP".equals(component) && "rollback before checkpoint".equals(message)) {
+ if (once.compareAndSet(false, true)) {
+ throw new RuntimeException("boom");
+ } else {
+ throw new AssertionError("has been rolled back twice");
+ }
+
+ }
+ }
+
+ @Override
+ public boolean isEnabled(String component) {
+ return "TP".equals(component);
+ }
+
+ @Override
+ public void close() {
+ }
+ };
+ try (Directory dir = newDirectory()) {
+ try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig().setInfoStream(stream)){
+ @Override
+ protected boolean isEnableTestPoints() {
+ return true;
+ }
+ }) {
+ }
+ } catch (RuntimeException x) {
Review comment:
why `x` ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org