Skip to content

Commit 5ed5bad

Browse files
hmottestadhmottestad
authored
GH-5059 remove calls to E(...) when the return value is already being checked (#5066)
2 parents ae61579 + e5faa2c commit 5ed5bad

8 files changed

Lines changed: 114 additions & 80 deletions

File tree

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbContextIdIterator.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,12 @@ public long[] next() {
104104
Varint.writeUnsigned(minKeyBuf, record[0]);
105105
minKeyBuf.flip();
106106
keyData.mv_data(minKeyBuf);
107-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET));
108-
if (lastResult != 0) {
107+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET);
108+
if (lastResult != MDB_SUCCESS) {
109109
// use MDB_SET_RANGE if key was deleted
110-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
110+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
111111
}
112-
if (lastResult != 0) {
112+
if (lastResult != MDB_SUCCESS) {
113113
closeInternal(false);
114114
return null;
115115
}
@@ -119,16 +119,16 @@ public long[] next() {
119119
}
120120

121121
if (fetchNext) {
122-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
122+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
123123
fetchNext = false;
124124
} else {
125125
if (minKeyBuf != null) {
126126
// set cursor to min key
127127
keyData.mv_data(minKeyBuf);
128-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
128+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
129129
} else {
130130
// set cursor to first item
131-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
131+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
132132
}
133133
}
134134

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbRecordIterator.java‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,12 @@ public long[] next() {
138138
index.toKey(minKeyBuf, quad[0], quad[1], quad[2], quad[3]);
139139
minKeyBuf.flip();
140140
keyData.mv_data(minKeyBuf);
141-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET));
142-
if (lastResult != 0) {
141+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET);
142+
if (lastResult != MDB_SUCCESS) {
143143
// use MDB_SET_RANGE if key was deleted
144-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
144+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
145145
}
146-
if (lastResult != 0) {
146+
if (lastResult != MDB_SUCCESS) {
147147
closeInternal(false);
148148
return null;
149149
}
@@ -153,16 +153,16 @@ public long[] next() {
153153
}
154154

155155
if (fetchNext) {
156-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
156+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
157157
fetchNext = false;
158158
} else {
159159
if (minKeyBuf != null) {
160160
// set cursor to min key
161161
keyData.mv_data(minKeyBuf);
162-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
162+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
163163
} else {
164164
// set cursor to first item
165-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
165+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
166166
}
167167
}
168168

@@ -172,7 +172,7 @@ public long[] next() {
172172
lastResult = MDB_NOTFOUND;
173173
} else if (groupMatcher != null && !groupMatcher.matches(keyData.mv_data())) {
174174
// value doesn't match search key/mask, fetch next value
175-
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
175+
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
176176
} else {
177177
// Matching value found
178178
index.keyToQuad(keyData.mv_data(), quad);
@@ -183,8 +183,6 @@ public long[] next() {
183183
}
184184
closeInternal(false);
185185
return null;
186-
} catch (IOException e) {
187-
throw new SailException(e);
188186
} finally {
189187
txnLock.unlockRead(stamp);
190188
}

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbSailStore.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -572,12 +572,14 @@ public void approve(Resource subj, IRI pred, Value obj, Resource ctx) throws Sai
572572

573573
@Override
574574
public void approveAll(Set<Statement> approved, Set<Resource> approvedContexts) {
575+
Statement last = null;
575576

576577
sinkStoreAccessLock.lock();
577578
try {
578579
startTransaction(true);
579580

580581
for (Statement statement : approved) {
582+
last = statement;
581583
Resource subj = statement.getSubject();
582584
IRI pred = statement.getPredicate();
583585
Value obj = statement.getObject();
@@ -604,13 +606,20 @@ public void approveAll(Set<Statement> approved, Set<Resource> approvedContexts)
604606
}
605607

606608
}
607-
} catch (IOException e) {
609+
} catch (IOException | RuntimeException e) {
608610
rollback();
611+
if (multiThreadingActive) {
612+
logger.error("Encountered an unexpected problem while trying to add a statement.", e);
613+
} else {
614+
logger.error(
615+
"Encountered an unexpected problem while trying to add a statement. Last statement that was attempted to be added: [ {} ]",
616+
last, e);
617+
}
618+
619+
if (e instanceof RuntimeException) {
620+
throw (RuntimeException) e;
621+
}
609622
throw new SailException(e);
610-
} catch (RuntimeException e) {
611-
rollback();
612-
logger.error("Encountered an unexpected problem while trying to add a statement", e);
613-
throw e;
614623
} finally {
615624
sinkStoreAccessLock.unlock();
616625
}

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbUtil.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static org.lwjgl.system.MemoryStack.stackPush;
1818
import static org.lwjgl.system.MemoryUtil.NULL;
19+
import static org.lwjgl.util.lmdb.LMDB.MDB_DBS_FULL;
1920
import static org.lwjgl.util.lmdb.LMDB.MDB_KEYEXIST;
2021
import static org.lwjgl.util.lmdb.LMDB.MDB_NOTFOUND;
2122
import static org.lwjgl.util.lmdb.LMDB.MDB_RDONLY;
@@ -39,12 +40,16 @@
3940
import org.lwjgl.system.Pointer;
4041
import org.lwjgl.util.lmdb.MDBCmpFuncI;
4142
import org.lwjgl.util.lmdb.MDBVal;
43+
import org.slf4j.Logger;
44+
import org.slf4j.LoggerFactory;
4245

4346
/**
4447
* Utility class for working with LMDB.
4548
*/
4649
final class LmdbUtil {
4750

51+
private static final Logger logger = LoggerFactory.getLogger(LmdbUtil.class);
52+
4853
/**
4954
* Minimum free space in an LMDB db before automatically resizing the map.
5055
*/
@@ -61,7 +66,9 @@ private LmdbUtil() {
6166

6267
static int E(int rc) throws IOException {
6368
if (rc != MDB_SUCCESS && rc != MDB_NOTFOUND && rc != MDB_KEYEXIST) {
64-
throw new IOException(mdb_strerror(rc));
69+
IOException ioException = new IOException(mdb_strerror(rc));
70+
logger.info("Possible LMDB error: {}", mdb_strerror(rc), ioException);
71+
throw ioException;
6572
}
6673
return rc;
6774
}
@@ -105,7 +112,7 @@ static <T> T transaction(long env, Transaction<T> transaction) throws IOExceptio
105112
int err;
106113
try {
107114
ret = transaction.exec(stack, txn);
108-
err = E(mdb_txn_commit(txn));
115+
err = mdb_txn_commit(txn);
109116
} catch (Throwable t) {
110117
mdb_txn_abort(txn);
111118
throw t;

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/PersistentSet.java‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import static org.eclipse.rdf4j.sail.lmdb.LmdbUtil.E;
1414
import static org.lwjgl.system.MemoryUtil.NULL;
15+
import static org.lwjgl.util.lmdb.LMDB.MDB_MAP_FULL;
1516
import static org.lwjgl.util.lmdb.LMDB.MDB_NEXT;
1617
import static org.lwjgl.util.lmdb.LMDB.MDB_NOOVERWRITE;
1718
import static org.lwjgl.util.lmdb.LMDB.MDB_SET;
@@ -24,6 +25,7 @@
2425
import static org.lwjgl.util.lmdb.LMDB.mdb_del;
2526
import static org.lwjgl.util.lmdb.LMDB.mdb_drop;
2627
import static org.lwjgl.util.lmdb.LMDB.mdb_put;
28+
import static org.lwjgl.util.lmdb.LMDB.mdb_strerror;
2729
import static org.lwjgl.util.lmdb.LMDB.mdb_txn_abort;
2830
import static org.lwjgl.util.lmdb.LMDB.mdb_txn_begin;
2931

@@ -43,12 +45,16 @@
4345
import org.lwjgl.PointerBuffer;
4446
import org.lwjgl.system.MemoryStack;
4547
import org.lwjgl.util.lmdb.MDBVal;
48+
import org.slf4j.Logger;
49+
import org.slf4j.LoggerFactory;
4650

4751
/**
4852
* A LMDB-based persistent set.
4953
*/
5054
class PersistentSet<T extends Serializable> extends AbstractSet<T> {
5155

56+
private static final Logger logger = LoggerFactory.getLogger(PersistentSet.class);
57+
5258
private PersistentSetFactory<T> factory;
5359
private final int dbi;
5460
private int size;
@@ -126,15 +132,35 @@ private synchronized boolean update(Object element, boolean add) throws IOExcept
126132
keyVal.mv_data(keyBuf);
127133

128134
if (add) {
129-
if (E(mdb_put(factory.writeTxn, dbi, keyVal, dataVal, MDB_NOOVERWRITE)) == MDB_SUCCESS) {
135+
int rc = mdb_put(factory.writeTxn, dbi, keyVal, dataVal, MDB_NOOVERWRITE);
136+
if (rc == MDB_SUCCESS) {
130137
size++;
131138
return true;
139+
} else if (rc == MDB_MAP_FULL) {
140+
factory.ensureResize();
141+
if (mdb_put(factory.writeTxn, dbi, keyVal, dataVal, MDB_NOOVERWRITE) == MDB_SUCCESS) {
142+
size++;
143+
return true;
144+
}
145+
return false;
146+
} else {
147+
logger.debug("Failed to add element due to error {}: {}", mdb_strerror(rc), element);
132148
}
133149
} else {
134150
// delete element
135-
if (mdb_del(factory.writeTxn, dbi, keyVal, dataVal) == MDB_SUCCESS) {
151+
int rc = mdb_del(factory.writeTxn, dbi, keyVal, dataVal);
152+
if (rc == MDB_SUCCESS) {
136153
size--;
137154
return true;
155+
} else if (rc == MDB_MAP_FULL) {
156+
factory.ensureResize();
157+
if (mdb_del(factory.writeTxn, dbi, keyVal, dataVal) == MDB_SUCCESS) {
158+
size--;
159+
return true;
160+
}
161+
return false;
162+
} else {
163+
logger.debug("Failed to remove element due to error {}: {}", mdb_strerror(rc), element);
138164
}
139165
}
140166
return false;

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/TripleStore.java‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ protected void filterUsedIds(Collection<Long> ids) throws IOException {
561561
keyBuf.clear();
562562
Varint.writeUnsigned(keyBuf, id);
563563
keyData.mv_data(keyBuf.flip());
564-
if (E(mdb_get(txn, contextsDbi, keyData, valueData)) == MDB_SUCCESS) {
564+
if (mdb_get(txn, contextsDbi, keyData, valueData) == MDB_SUCCESS) {
565565
it.remove();
566566
}
567567
}
@@ -587,15 +587,15 @@ protected void filterUsedIds(Collection<Long> ids) throws IOException {
587587

588588
if (fullScan) {
589589
long[] quad = new long[4];
590-
int rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_FIRST));
590+
int rc = mdb_cursor_get(cursor, keyData, valueData, MDB_FIRST);
591591
while (rc == MDB_SUCCESS && !ids.isEmpty()) {
592592
index.keyToQuad(keyData.mv_data(), quad);
593593
ids.remove(quad[0]);
594594
ids.remove(quad[1]);
595595
ids.remove(quad[2]);
596596
ids.remove(quad[3]);
597597

598-
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
598+
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
599599
}
600600
} else {
601601
for (Iterator<Long> it = ids.iterator(); it.hasNext();) {
@@ -625,15 +625,15 @@ protected void filterUsedIds(Collection<Long> ids) throws IOException {
625625

626626
// set cursor to min key
627627
keyData.mv_data(keyBuf);
628-
int rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
628+
int rc = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
629629
boolean exists = false;
630-
while (!exists && rc == 0) {
630+
while (!exists && rc == MDB_SUCCESS) {
631631
if (mdb_cmp(txn, dbi, keyData, maxKey) > 0) {
632632
// id was not found
633633
break;
634634
} else if (!matcher.matches(keyData.mv_data())) {
635635
// value doesn't match search key/mask, fetch next value
636-
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
636+
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
637637
} else {
638638
exists = true;
639639
}
@@ -708,24 +708,24 @@ protected double cardinality(long subj, long pred, long obj, long context) throw
708708

709709
// set cursor to min key
710710
keyData.mv_data(keyBuf);
711-
int rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
712-
if (rc != 0 || mdb_cmp(txn, dbi, keyData, maxKey) >= 0) {
711+
int rc = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
712+
if (rc != MDB_SUCCESS || mdb_cmp(txn, dbi, keyData, maxKey) >= 0) {
713713
break;
714714
} else {
715715
Varint.readListUnsigned(keyData.mv_data(), s.minValues);
716716
}
717717

718718
// set cursor to max key
719719
keyData.mv_data(maxKeyBuf);
720-
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
721-
if (rc != 0) {
720+
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
721+
if (rc != MDB_SUCCESS) {
722722
// directly go to last value
723-
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_LAST));
723+
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_LAST);
724724
} else {
725725
// go to previous value of selected key
726-
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_PREV));
726+
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_PREV);
727727
}
728-
if (rc == 0) {
728+
if (rc == MDB_SUCCESS) {
729729
Varint.readListUnsigned(keyData.mv_data(), s.maxValues);
730730
// this is required to correctly estimate the range size at a later point
731731
s.startValues[s.MAX_BUCKETS] = s.maxValues;
@@ -747,7 +747,7 @@ protected double cardinality(long subj, long pred, long obj, long context) throw
747747
keyData.mv_data(keyBuf);
748748

749749
int currentSamplesCount = 0;
750-
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
750+
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
751751
while (rc == MDB_SUCCESS && currentSamplesCount < s.MAX_SAMPLES_PER_BUCKET) {
752752
if (mdb_cmp(txn, dbi, keyData, maxKey) >= 0) {
753753
endOfRange = true;
@@ -776,8 +776,8 @@ protected double cardinality(long subj, long pred, long obj, long context) throw
776776
}
777777
}
778778
}
779-
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
780-
if (rc != 0) {
779+
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
780+
if (rc != MDB_SUCCESS) {
781781
// no more elements are available
782782
endOfRange = true;
783783
}
@@ -873,14 +873,14 @@ public boolean storeTriple(long subj, long pred, long obj, long context, boolean
873873
return recordCache.storeRecord(quad, explicit);
874874
}
875875

876-
int rc = E(mdb_put(writeTxn, mainIndex.getDB(explicit), keyVal, dataVal, MDB_NOOVERWRITE));
876+
int rc = mdb_put(writeTxn, mainIndex.getDB(explicit), keyVal, dataVal, MDB_NOOVERWRITE);
877877
if (rc != MDB_SUCCESS && rc != MDB_KEYEXIST) {
878878
throw new IOException(mdb_strerror(rc));
879879
}
880880
stAdded = rc == MDB_SUCCESS;
881881
boolean foundImplicit = false;
882882
if (explicit && stAdded) {
883-
foundImplicit = E(mdb_del(writeTxn, mainIndex.getDB(false), keyVal, dataVal)) == MDB_SUCCESS;
883+
foundImplicit = mdb_del(writeTxn, mainIndex.getDB(false), keyVal, dataVal) == MDB_SUCCESS;
884884
}
885885

886886
if (stAdded) {
@@ -920,7 +920,7 @@ private void incrementContext(MemoryStack stack, long context) throws IOExceptio
920920
idVal.mv_data(bb);
921921
MDBVal dataVal = MDBVal.calloc(stack);
922922
long newCount = 1;
923-
if (E(mdb_get(writeTxn, contextsDbi, idVal, dataVal)) == MDB_SUCCESS) {
923+
if (mdb_get(writeTxn, contextsDbi, idVal, dataVal) == MDB_SUCCESS) {
924924
// update count
925925
newCount = Varint.readUnsigned(dataVal.mv_data()) + 1;
926926
}
@@ -944,7 +944,7 @@ private boolean decrementContext(MemoryStack stack, long context) throws IOExcep
944944
bb.flip();
945945
idVal.mv_data(bb);
946946
MDBVal dataVal = MDBVal.calloc(stack);
947-
if (E(mdb_get(writeTxn, contextsDbi, idVal, dataVal)) == MDB_SUCCESS) {
947+
if (mdb_get(writeTxn, contextsDbi, idVal, dataVal) == MDB_SUCCESS) {
948948
// update count
949949
long newCount = Varint.readUnsigned(dataVal.mv_data()) - 1;
950950
if (newCount <= 0) {

0 commit comments

Comments
 (0)