Merge lp:~pbeaman/akiban-persistit/harden-CLI into lp:akiban-persistit

Proposed by Peter Beaman
Status: Merged
Approved by: Nathan Williams
Approved revision: 360
Merged at revision: 364
Proposed branch: lp:~pbeaman/akiban-persistit/harden-CLI
Merge into: lp:akiban-persistit
Diff against target: 278 lines (+117/-75)
3 files modified
src/main/java/com/persistit/Buffer.java (+74/-44)
src/main/java/com/persistit/IntegrityCheck.java (+38/-31)
src/main/java/com/persistit/Management.java (+5/-0)
To merge this branch: bzr merge lp:~pbeaman/akiban-persistit/harden-CLI
Reviewer Review Type Date Requested Status
Akiban Build User Needs Fixing
Peter Beaman Approve
Nathan Williams Approve
Review via email: mp+122286@code.launchpad.net

Description of the change

Fix AIOOBEs caused by invalid bytes in pages being examined by IntegrityCheck and CLI#pview, bug https://bugs.launchpad.net/akiban-persistit/+bug/1044397. These fixes were added to facilitate examining a corrupt database.

To post a comment you must log in.
Revision history for this message
Nathan Williams (nwilliams) wrote :

Simple enough.

review: Approve
Revision history for this message
Akiban Build User (build-akiban) wrote :

There were 2 failures during build/test:

* job persistit-build failed at build number 417: http://172.16.20.104:8080/job/persistit-build/417/

* view must-pass failed: persistit-build is yellow

review: Needs Fixing
Revision history for this message
Peter Beaman (pbeaman) wrote :
Revision history for this message
Peter Beaman (pbeaman) wrote :

Approving again since the fix for IntegrityCheckTest has been merged.

review: Approve
Revision history for this message
Akiban Build User (build-akiban) wrote :

There were 2 failures during build/test:

* job persistit-build failed at build number 422: http://172.16.20.104:8080/job/persistit-build/422/

* view must-pass failed: persistit-build is yellow

review: Needs Fixing
Revision history for this message
Nathan Williams (nwilliams) wrote :

Another branch that should fix the last failure (unrelated to this change) is now in. Trying again.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/main/java/com/persistit/Buffer.java'
2--- src/main/java/com/persistit/Buffer.java 2012-08-28 14:29:50 +0000
3+++ src/main/java/com/persistit/Buffer.java 2012-08-31 14:15:23 +0000
4@@ -3858,7 +3858,7 @@
5 boolean elision = false;
6 for (int index = 0; index < records.length; index++) {
7 final RecordInfo r = records[index];
8- r.getKeyState().copyTo(key);
9+
10 String mark = " ";
11 boolean selected = all | index < contextLines || index >= records.length - contextLines;
12 if (foundPointerRecord >= 0) {
13@@ -3870,21 +3870,40 @@
14 }
15
16 if (selected) {
17+ final String keyString;
18+ final String valueString;
19+
20+ if (r.getKeyState() != null) {
21+ r.getKeyState().copyTo(key);
22+ keyString = Util.abridge(key.toString(), maxKeyDisplayLength);
23+ } else {
24+ keyString = "*error*";
25+ }
26+
27+ if (isDataPage() && r.getValueState() != null) {
28+ r.getValueState().copyTo(value);
29+ valueString = Util.abridge(value.toString(), maxValueDisplayLength);
30+ } else {
31+ valueString = "*error*";
32+ }
33+
34 if (elision) {
35 sb.append(String.format("\n ..."));
36 elision = false;
37 }
38+
39 if (isDataPage()) {
40 r.getValueState().copyTo(value);
41- sb.append(String.format("\n%s %5d: db=%3d ebc=%3d tb=%,5d [%,d]%s=[%,d]%s", mark, r
42- .getKbOffset(), r.getDb(), r.getEbc(), r.getTbOffset(), r.getKLength(), Util
43- .abridge(key.toString(), maxKeyDisplayLength),
44- r.getValueState().getEncodedBytes().length, Util.abridge(value.toString(),
45- maxValueDisplayLength)));
46+ sb.append(String.format("\n%s %5d: db=%3d ebc=%3d tb=%,5d [%,d]%s=[%,d]%s", mark,
47+ r.getKbOffset(), r.getDb(), r.getEbc(), r.getTbOffset(), r.getKLength(), keyString,
48+ r.getValueState().getEncodedBytes().length, valueString));
49 } else {
50- sb.append(String.format("\n%s %5d: db=%3d ebc=%3d tb=%,5d [%,d]%s->%,d", mark,
51- r.getKbOffset(), r.getDb(), r.getEbc(), r.getTbOffset(), r.getKLength(),
52- Util.abridge(key.toString(), maxKeyDisplayLength), r.getPointerValue()));
53+ sb.append(String.format("\n%s %5d: db=%3d ebc=%3d tb=%,5d [%,d]%s->%,d %s", mark,
54+ r.getKbOffset(), r.getDb(), r.getEbc(), r.getTbOffset(), r.getKLength(), keyString,
55+ r.getPointerValue()));
56+ }
57+ if (r.getError() != null) {
58+ sb.append(String.format(" !! %s", r.getError()));
59 }
60 } else {
61 elision = true;
62@@ -3954,43 +3973,54 @@
63 result = new ManagementImpl.RecordInfo[count];
64 int n = 0;
65 for (int p = KEY_BLOCK_START; p < _keyBlockEnd; p += KEYBLOCK_LENGTH) {
66- final int kbData = getInt(p);
67- final int db = decodeKeyBlockDb(kbData);
68- final int ebc = decodeKeyBlockEbc(kbData);
69- final int tail = decodeKeyBlockTail(kbData);
70- final int tbData = tail != 0 ? getInt(tail) : 0;
71- final int size = decodeTailBlockSize(tbData);
72- final int klength = decodeTailBlockKLength(tbData);
73- final boolean inUse = decodeTailBlockInUse(tbData);
74-
75 final ManagementImpl.RecordInfo rec = new ManagementImpl.RecordInfo();
76- rec._kbOffset = p;
77- rec._tbOffset = tail;
78- rec._ebc = ebc;
79- rec._db = db;
80- rec._klength = klength;
81- rec._size = size;
82- rec._inUse = inUse;
83-
84- final byte[] kbytes = key.getEncodedBytes();
85- kbytes[ebc] = (byte) db;
86- System.arraycopy(_bytes, tail + _tailHeaderSize, kbytes, ebc + 1, klength);
87- key.setEncodedSize(ebc + 1 + klength);
88- rec._key = new KeyState(key);
89-
90- if (isIndexPage()) {
91- rec._pointerValue = getInt(tail + 4);
92- } else {
93- int vsize = size - _tailHeaderSize - klength;
94- if (vsize < 0)
95- vsize = 0;
96- value.putEncodedBytes(_bytes, tail + _tailHeaderSize + klength, vsize);
97- if (value.isDefined() && (value.getEncodedBytes()[0] & 0xFF) == LONGREC_TYPE) {
98- value.setLongRecordMode(true);
99+ try {
100+ rec._kbOffset = p;
101+
102+ final int kbData = getInt(p);
103+ final int db = decodeKeyBlockDb(kbData);
104+ final int ebc = decodeKeyBlockEbc(kbData);
105+ final int tail = decodeKeyBlockTail(kbData);
106+ rec._tbOffset = tail;
107+ rec._ebc = ebc;
108+ rec._db = db;
109+
110+ final int tbData = tail != 0 ? getInt(tail) : 0;
111+ final int size = decodeTailBlockSize(tbData);
112+ final int klength = decodeTailBlockKLength(tbData);
113+ final boolean inUse = decodeTailBlockInUse(tbData);
114+ rec._klength = klength;
115+ rec._size = size;
116+ rec._inUse = inUse;
117+
118+ final byte[] kbytes = key.getEncodedBytes();
119+ kbytes[ebc] = (byte) db;
120+ System.arraycopy(_bytes, tail + _tailHeaderSize, kbytes, ebc + 1, klength);
121+ key.setEncodedSize(ebc + 1 + klength);
122+ rec._key = new KeyState(key);
123+
124+ if (isIndexPage()) {
125+ rec._pointerValue = getInt(tail + 4);
126 } else {
127- value.setLongRecordMode(false);
128- }
129- rec._value = new ValueState(value);
130+ int vsize = size - _tailHeaderSize - klength;
131+ if (vsize < 0)
132+ vsize = 0;
133+ value.putEncodedBytes(_bytes, tail + _tailHeaderSize + klength, vsize);
134+ if (value.isDefined() && (value.getEncodedBytes()[0] & 0xFF) == LONGREC_TYPE) {
135+ value.setLongRecordMode(true);
136+ } else {
137+ value.setLongRecordMode(false);
138+ }
139+ rec._value = new ValueState(value);
140+ }
141+ } catch (final Exception e) {
142+ rec._error = e.toString();
143+ if (rec._key == null) {
144+ rec._key = new KeyState(new Key(_persistit));
145+ }
146+ if (rec._value == null) {
147+ rec._value = new ValueState(new Value(_persistit));
148+ }
149 }
150 result[n++] = rec;
151 }
152
153=== modified file 'src/main/java/com/persistit/IntegrityCheck.java'
154--- src/main/java/com/persistit/IntegrityCheck.java 2012-08-24 13:57:19 +0000
155+++ src/main/java/com/persistit/IntegrityCheck.java 2012-08-31 14:15:23 +0000
156@@ -831,20 +831,6 @@
157 }
158 _currentTree = null;
159 }
160- if (_counters._indexHoleCount > 0) {
161- postMessage(
162- " Tree " + resourceName(tree) + " has "
163- + plural((int) _counters._indexHoleCount, "unindexed page"), LOG_NORMAL);
164- if (_fixHoles) {
165- int offered = 0;
166- for (final CleanupIndexHole hole : _holes) {
167- if (_persistit.getCleanupManager().offer(hole)) {
168- offered++;
169- }
170- }
171- postMessage(" - enqueued " + offered + " for repair", LOG_NORMAL);
172- }
173- }
174 } finally {
175 tree.release();
176 }
177@@ -852,6 +838,21 @@
178 faults = _faults.size() - faults;
179 treeCounters.difference(_counters);
180
181+ if (_counters._indexHoleCount > 0) {
182+ postMessage(
183+ " Tree " + resourceName(tree) + " has "
184+ + plural((int) _counters._indexHoleCount, "unindexed page"), LOG_NORMAL);
185+ if (_fixHoles) {
186+ int offered = 0;
187+ for (final CleanupIndexHole hole : _holes) {
188+ if (_persistit.getCleanupManager().offer(hole)) {
189+ offered++;
190+ }
191+ }
192+ postMessage(" - enqueued " + offered + " for repair", LOG_NORMAL);
193+ }
194+ }
195+
196 if (_csv) {
197 postMessage(String.format("%s,%d,%s", messageStart, faults, treeCounters.toCSV()), LOG_NORMAL);
198 } else {
199@@ -1136,26 +1137,29 @@
200 addFault("Buffer contains wrong page " + buffer.getPageAddress(), page, level, 0);
201 return false;
202 }
203-
204- if (buffer.isDataPage() || buffer.isIndexPage()) {
205- final long mvvCount = _counters._mvvCount;
206- final PersistitException ipse = buffer.verify(key, _visitor);
207- if (ipse != null) {
208- addFault(ipse.getMessage(), page, level, 0);
209- key.clear();
210- return false;
211- }
212- if (_counters._mvvCount > mvvCount) {
213- _counters._mvvPageCount++;
214- if (_prune && !_currentVolume.isReadOnly() && _counters._pruningErrorCount < MAX_PRUNING_ERRORS) {
215- try {
216- buffer.pruneMvvValues(tree, true);
217- _counters._prunedPageCount++;
218- } catch (final PersistitException e) {
219- _counters._pruningErrorCount++;
220+ try {
221+ if (buffer.isDataPage() || buffer.isIndexPage()) {
222+ final long mvvCount = _counters._mvvCount;
223+ final PersistitException ipse = buffer.verify(key, _visitor);
224+ if (ipse != null) {
225+ addFault(ipse.getMessage(), page, level, 0);
226+ key.clear();
227+ return false;
228+ }
229+ if (_counters._mvvCount > mvvCount) {
230+ _counters._mvvPageCount++;
231+ if (_prune && !_currentVolume.isReadOnly() && _counters._pruningErrorCount < MAX_PRUNING_ERRORS) {
232+ try {
233+ buffer.pruneMvvValues(tree, true);
234+ _counters._prunedPageCount++;
235+ } catch (final PersistitException e) {
236+ _counters._pruningErrorCount++;
237+ }
238 }
239 }
240 }
241+ } catch (Exception e) {
242+ addFault(e.toString(), page, level, 0);
243 }
244 return true;
245 }
246@@ -1209,6 +1213,9 @@
247
248 fromPage = longPage;
249 longPage = longBuffer.getRightSibling();
250+ } catch (Exception e) {
251+ addFault(e.toString() + " while verifying long record page " + longPage, page, 0, foundAt);
252+ break;
253 } finally {
254 if (longBuffer != null)
255 longBuffer.release();
256
257=== modified file 'src/main/java/com/persistit/Management.java'
258--- src/main/java/com/persistit/Management.java 2012-08-24 13:57:19 +0000
259+++ src/main/java/com/persistit/Management.java 2012-08-31 14:15:23 +0000
260@@ -817,6 +817,7 @@
261 boolean _inUse;
262 KeyState _key;
263 ValueState _value;
264+ String _error;
265 //
266 // Only used for index pages.
267 //
268@@ -879,6 +880,10 @@
269 public long getGarbageRightPage() {
270 return _garbageRightPage;
271 }
272+
273+ public String getError() {
274+ return _error;
275+ }
276 }
277
278 /**

Subscribers

People subscribed via source and target branches