Merge lp:~nwilliams/akiban-persistit/fix_912514_fetchAndRemove_2 into lp:akiban-persistit

Proposed by Nathan Williams
Status: Superseded
Proposed branch: lp:~nwilliams/akiban-persistit/fix_912514_fetchAndRemove_2
Merge into: lp:akiban-persistit
Diff against target: 1043 lines (+390/-292)
11 files modified
bench/pom.xml (+0/-10)
core/pom.xml (+0/-194)
examples/FindFile/build.xml (+4/-4)
examples/HelloWorld/build.xml (+4/-4)
examples/PersistitMapDemo/build.xml (+4/-4)
examples/SimpleBench/build.xml (+4/-4)
examples/SimpleDemo/build.xml (+4/-4)
examples/SimpleTransaction/build.xml (+4/-4)
pom.xml (+181/-8)
src/main/java/com/persistit/Exchange.java (+71/-56)
src/test/java/com/persistit/Bug912514Test.java (+114/-0)
To merge this branch: bzr merge lp:~nwilliams/akiban-persistit/fix_912514_fetchAndRemove_2
Reviewer Review Type Date Requested Status
Peter Beaman Pending
Review via email: mp+107855@code.launchpad.net

This proposal has been superseded by a proposal from 2012-05-29.

Description of the change

Fix Exchange store and fetchAndRemove to work correctly both inside and out of a transaction.

The vast majority of this is Peter's original branch. The last few commits were refactorings, to eliminate duplicate code, that I noticed were possible and suggested on the merge prop.

Original branch and description:
lp:~pbeaman/akiban-persistit/fix_912514_fetchAndRemove

The basic strategy to fix this bug is to used a the ThreadLocal-based cached Value object, rather than Exchange._spareValue, as the MVV holding value. The major change was to remove the "_" from instances of _spareValue in storeInternal. However, there were other changes required to reenable StoreOption.FETCH | StoreOption.MVCC, etc.

New test Bug912514 test tests some of the cases. I will also (and have not yet) run PersistitMapStressTest which is where we first observed the bug.

To post a comment you must log in.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== removed directory 'bench'
2=== removed file 'bench/pom.xml'
3--- bench/pom.xml 2011-02-01 16:48:36 +0000
4+++ bench/pom.xml 1970-01-01 00:00:00 +0000
5@@ -1,10 +0,0 @@
6-<?xml version="1.0"?>
7-<project>
8- <modelVersion>4.0.0</modelVersion>
9- <groupId>com.akiban</groupId>
10- <artifactId>akiban-persistit-bench</artifactId>
11- <packaging>jar</packaging>
12- <name>akiban-persistit-bench</name>
13- <version>0.0-SNAPSHOT</version>
14-</project>
15-
16
17=== removed directory 'core'
18=== removed file 'core/pom.xml'
19--- core/pom.xml 2012-05-29 14:05:18 +0000
20+++ core/pom.xml 1970-01-01 00:00:00 +0000
21@@ -1,194 +0,0 @@
22-<?xml version="1.0"?>
23-<project>
24- <modelVersion>4.0.0</modelVersion>
25- <parent>
26- <groupId>com.akiban</groupId>
27- <artifactId>akiban-persistit</artifactId>
28- <version>3.1.1-SNAPSHOT</version>
29- </parent>
30- <artifactId>akiban-persistit-core</artifactId>
31- <packaging>jar</packaging>
32- <name>akiban-persistit-core</name>
33- <version>3.1.1-SNAPSHOT</version>
34- <dependencies>
35- <dependency>
36- <groupId>junit</groupId>
37- <artifactId>junit</artifactId>
38- <version>4.8.1</version>
39- <scope>test</scope>
40- </dependency>
41- <dependency>
42- <groupId>org.slf4j</groupId>
43- <artifactId>slf4j-api</artifactId>
44- <version>1.6.1</version>
45- </dependency>
46- <dependency>
47- <groupId>commons-logging</groupId>
48- <artifactId>commons-logging</artifactId>
49- <version>1.1</version>
50- <exclusions>
51- <exclusion>
52- <groupId>logkit</groupId>
53- <artifactId>logkit</artifactId>
54- </exclusion>
55- <exclusion>
56- <groupId>avalon-framework</groupId>
57- <artifactId>avalon-framework</artifactId>
58- </exclusion>
59- <exclusion>
60- <groupId>javax.servlet</groupId>
61- <artifactId>servlet-api</artifactId>
62- </exclusion>
63- </exclusions>
64- </dependency>
65- </dependencies>
66- <build>
67- <finalName>${project.artifactId}-${project.version}${BZR_REVISION}</finalName>
68- <resources>
69- <resource>
70- <directory>src/main/resources</directory>
71- <filtering>true</filtering>
72- <includes>
73- <include>com/persistit/persistit_version</include>
74- </includes>
75- </resource>
76- <resource>
77- <directory>src/main/resources</directory>
78- <filtering>false</filtering>
79- <excludes>
80- <exclude>com/persistit/persistit_version</exclude>
81- </excludes>
82- </resource>
83- </resources>
84- <plugins>
85- <plugin>
86- <groupId>org.apache.maven.plugins</groupId>
87- <artifactId>maven-resources-plugin</artifactId>
88- <version>2.4.3</version>
89- </plugin>
90- <plugin>
91- <groupId>org.codehaus.mojo</groupId>
92- <artifactId>build-helper-maven-plugin</artifactId>
93- <version>1.5</version>
94- <executions>
95- <execution>
96- <id>reserve-network-port</id>
97- <goals>
98- <goal>reserve-network-port</goal>
99- </goals>
100- <phase>process-resources</phase>
101- <configuration>
102- <portNames>
103- <portName>rmiport</portName>
104- </portNames>
105- </configuration>
106- </execution>
107- </executions>
108- </plugin>
109- <plugin>
110- <groupId>org.apache.maven.plugins</groupId>
111- <artifactId>maven-compiler-plugin</artifactId>
112- <version>2.0.2</version>
113- <configuration>
114- <source>1.6</source>
115- <target>1.6</target>
116- </configuration>
117- </plugin>
118- <plugin>
119- <groupId>org.apache.maven.plugins</groupId>
120- <artifactId>maven-source-plugin</artifactId>
121- <version>2.0.4</version>
122- <executions>
123- <execution>
124- <id>attach-sources</id>
125- <phase>package</phase>
126- <goals>
127- <goal>jar</goal>
128- </goals>
129- </execution>
130- </executions>
131- </plugin>
132- <plugin>
133- <groupId>org.apache.maven.plugins</groupId>
134- <artifactId>maven-surefire-plugin</artifactId>
135- <version>2.4.1</version>
136- <configuration>
137- <argLine>-Xmx640m -Xms128m -Drmiport=${rmiport} -Xrunjdwp:transport=dt_socket,address=8000,suspend=n,server=y</argLine>
138- <includes>
139- <include>**/*Test.java</include>
140- <include>**/*Test?.java</include>
141- </includes>
142- </configuration>
143- </plugin>
144- <plugin>
145- <groupId>org.codehaus.mojo</groupId>
146- <artifactId>rmic-maven-plugin</artifactId>
147- <version>1.0</version>
148- <configuration>
149- <outputDirectory>target/classes/</outputDirectory>
150- </configuration>
151- <executions>
152- <execution>
153- <id>rmi compilation</id>
154- <goals>
155- <goal>rmic</goal>
156- </goals>
157- </execution>
158- </executions>
159- </plugin>
160- <plugin>
161- <artifactId>maven-assembly-plugin</artifactId>
162- <configuration>
163- <descriptors>
164- <descriptor>src/main/resources/assembly_descriptor.xml</descriptor>
165- </descriptors>
166- </configuration>
167- <executions>
168- <execution>
169- <id>make-assembly</id> <!-- This is used for inheritance merges ?? -->
170- <phase>install</phase> <!-- Append to the install phase -->
171- <goals>
172- <goal>single</goal>
173- </goals>
174- </execution>
175- </executions>
176- </plugin>
177- <plugin>
178- <groupId>org.codehaus.mojo</groupId>
179- <artifactId>findbugs-maven-plugin</artifactId>
180- <version>1.2</version>
181- <configuration>
182- <findbugsXmlOutput>true</findbugsXmlOutput>
183- <findbugsXmlWithMessages>true</findbugsXmlWithMessages>
184- <xmlOutput>true</xmlOutput>
185- <excludeFilterFile>findbugs-exclude.xml</excludeFilterFile>
186- </configuration>
187- </plugin>
188- <plugin>
189- <groupId>com.atlassian.maven.plugins</groupId>
190- <artifactId>maven-clover2-plugin</artifactId>
191- <version>3.0.1</version>
192- <configuration>
193- <generateHistorical>true</generateHistorical>
194- <historyDir>/clover/history</historyDir>
195- <license>${clover.license}</license>
196- <generateXml>true</generateXml>
197- </configuration>
198- </plugin>
199- </plugins>
200- </build>
201- <reporting>
202- <plugins>
203- <plugin>
204- <groupId>org.apache.maven.plugins</groupId>
205- <artifactId>maven-surefire-report-plugin</artifactId>
206- <version>2.4.2</version>
207- </plugin>
208- <plugin>
209- <groupId>org.apache.maven.plugins</groupId>
210- <artifactId>maven-javadoc-plugin</artifactId>
211- <version>2.8</version>
212- </plugin>
213- </plugins>
214- </reporting>
215-</project>
216
217=== modified file 'examples/FindFile/build.xml'
218--- examples/FindFile/build.xml 2012-05-25 18:50:59 +0000
219+++ examples/FindFile/build.xml 2012-05-29 19:27:22 +0000
220@@ -31,8 +31,8 @@
221 srcdir="."
222 destdir=".">
223 <classpath>
224- <fileset dir="../../core/target">
225- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
226+ <fileset dir="../../target">
227+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
228 </fileset>
229 </classpath>
230 </javac>
231@@ -40,8 +40,8 @@
232 <target name="run" depends="compile">
233 <java fork="true" classname="FindFile">
234 <classpath>
235- <fileset dir="../../core/target">
236- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
237+ <fileset dir="../../target">
238+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
239 </fileset>
240 <pathelement location="." />
241 </classpath>
242
243=== modified file 'examples/HelloWorld/build.xml'
244--- examples/HelloWorld/build.xml 2012-05-25 18:50:59 +0000
245+++ examples/HelloWorld/build.xml 2012-05-29 19:27:22 +0000
246@@ -29,8 +29,8 @@
247 srcdir="."
248 destdir=".">
249 <classpath>
250- <fileset dir="../../core/target">
251- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
252+ <fileset dir="../../target">
253+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
254 </fileset>
255 <pathelement location="." />
256 </classpath>
257@@ -40,8 +40,8 @@
258 <target name="run" depends="compile">
259 <java classname="HelloWorld" fork="true">
260 <classpath>
261- <fileset dir="../../core/target">
262- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
263+ <fileset dir="../../target">
264+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
265 </fileset>
266 <pathelement location="." />
267 </classpath>
268
269=== modified file 'examples/PersistitMapDemo/build.xml'
270--- examples/PersistitMapDemo/build.xml 2012-05-25 18:50:59 +0000
271+++ examples/PersistitMapDemo/build.xml 2012-05-29 19:27:22 +0000
272@@ -29,8 +29,8 @@
273 srcdir="."
274 destdir=".">
275 <classpath>
276- <fileset dir="../../core/target">
277- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
278+ <fileset dir="../../target">
279+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
280 </fileset>
281 <pathelement location="." />
282 </classpath>
283@@ -40,8 +40,8 @@
284 <target name="run" depends="compile">
285 <java classname="PersistitMapDemo" fork="true" >
286 <classpath>
287- <fileset dir="../../core/target">
288- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
289+ <fileset dir="../../target">
290+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
291 </fileset>
292 <pathelement location="." />
293 </classpath>
294
295=== modified file 'examples/SimpleBench/build.xml'
296--- examples/SimpleBench/build.xml 2012-05-25 18:50:59 +0000
297+++ examples/SimpleBench/build.xml 2012-05-29 19:27:22 +0000
298@@ -29,8 +29,8 @@
299 srcdir="."
300 destdir=".">
301 <classpath>
302- <fileset dir="../../core/target">
303- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
304+ <fileset dir="../../target">
305+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
306 </fileset>
307 </classpath>
308 </javac>
309@@ -40,8 +40,8 @@
310 <target name="run" depends="compile">
311 <java classname="SimpleBench" fork="true">
312 <classpath>
313- <fileset dir="../../core/target">
314- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
315+ <fileset dir="../../target">
316+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
317 </fileset>
318 <pathelement location="." />
319 </classpath>
320
321=== modified file 'examples/SimpleDemo/build.xml'
322--- examples/SimpleDemo/build.xml 2012-05-25 18:50:59 +0000
323+++ examples/SimpleDemo/build.xml 2012-05-29 19:27:22 +0000
324@@ -29,8 +29,8 @@
325 srcdir="."
326 destdir=".">
327 <classpath>
328- <fileset dir="../../core/target">
329- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
330+ <fileset dir="../../target">
331+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
332 </fileset>
333 </classpath>
334 </javac>
335@@ -39,8 +39,8 @@
336 <target name="run" depends="compile">
337 <java classname="SimpleDemo" fork="true">
338 <classpath>
339- <fileset dir="../../core/target">
340- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
341+ <fileset dir="../../target">
342+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
343 </fileset>
344 <pathelement location="." />
345 </classpath>
346
347=== modified file 'examples/SimpleTransaction/build.xml'
348--- examples/SimpleTransaction/build.xml 2012-05-25 18:50:59 +0000
349+++ examples/SimpleTransaction/build.xml 2012-05-29 19:27:22 +0000
350@@ -29,8 +29,8 @@
351 srcdir="."
352 destdir=".">
353 <classpath>
354- <fileset dir="../../core/target">
355- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
356+ <fileset dir="../../target">
357+ <include name="akiban-persistit-*-with-dependencies-and-tests.jar"/>
358 </fileset>
359 </classpath>
360 </javac>
361@@ -39,8 +39,8 @@
362 <target name="run" depends="compile">
363 <java classname="SimpleTransaction" fork="true">
364 <classpath>
365- <fileset dir="../../core/target">
366- <include name="akiban-persistit-core-*-with-dependencies-and-tests.jar"/>
367+ <fileset dir="../../target">
368+ <include name="akiban-persistit*-with-dependencies-and-tests.jar"/>
369 </fileset>
370 <pathelement location="." />
371 </classpath>
372
373=== modified file 'pom.xml'
374--- pom.xml 2012-05-29 18:51:35 +0000
375+++ pom.xml 2012-05-29 19:27:22 +0000
376@@ -3,9 +3,9 @@
377 <modelVersion>4.0.0</modelVersion>
378 <groupId>com.akiban</groupId>
379 <artifactId>akiban-persistit</artifactId>
380- <packaging>pom</packaging>
381 <name>akiban-persistit</name>
382 <version>3.1.1-SNAPSHOT</version>
383+ <packaging>jar</packaging>
384 <properties>
385 <!-- this is the default version number, Jenkins job sets this to the official number -->
386 <BZR_REVISION></BZR_REVISION>
387@@ -24,20 +24,179 @@
388 <url>http://akiban.artifactoryonline.com/akiban/libs-releases-local</url>
389 </repository>
390 </distributionManagement>
391- <modules>
392- <module>core</module>
393- <module>bench</module>
394- </modules>
395+ <dependencies>
396+ <dependency>
397+ <groupId>junit</groupId>
398+ <artifactId>junit</artifactId>
399+ <version>4.8.1</version>
400+ <scope>test</scope>
401+ </dependency>
402+ <dependency>
403+ <groupId>org.slf4j</groupId>
404+ <artifactId>slf4j-api</artifactId>
405+ <version>1.6.1</version>
406+ </dependency>
407+ <dependency>
408+ <groupId>commons-logging</groupId>
409+ <artifactId>commons-logging</artifactId>
410+ <version>1.1</version>
411+ <exclusions>
412+ <exclusion>
413+ <groupId>logkit</groupId>
414+ <artifactId>logkit</artifactId>
415+ </exclusion>
416+ <exclusion>
417+ <groupId>avalon-framework</groupId>
418+ <artifactId>avalon-framework</artifactId>
419+ </exclusion>
420+ <exclusion>
421+ <groupId>javax.servlet</groupId>
422+ <artifactId>servlet-api</artifactId>
423+ </exclusion>
424+ </exclusions>
425+ </dependency>
426+ </dependencies>
427 <build>
428+ <finalName>${project.artifactId}-${project.version}${BZR_REVISION}</finalName>
429+ <resources>
430+ <resource>
431+ <directory>src/main/resources</directory>
432+ <filtering>true</filtering>
433+ <includes>
434+ <include>com/persistit/persistit_version</include>
435+ </includes>
436+ </resource>
437+ <resource>
438+ <directory>src/main/resources</directory>
439+ <filtering>false</filtering>
440+ <excludes>
441+ <exclude>com/persistit/persistit_version</exclude>
442+ </excludes>
443+ </resource>
444+ </resources>
445 <plugins>
446 <plugin>
447+ <groupId>org.apache.maven.plugins</groupId>
448+ <artifactId>maven-resources-plugin</artifactId>
449+ <version>2.4.3</version>
450+ </plugin>
451+ <plugin>
452+ <groupId>org.codehaus.mojo</groupId>
453+ <artifactId>build-helper-maven-plugin</artifactId>
454+ <version>1.5</version>
455+ <executions>
456+ <execution>
457+ <id>reserve-network-port</id>
458+ <goals>
459+ <goal>reserve-network-port</goal>
460+ </goals>
461+ <phase>process-resources</phase>
462+ <configuration>
463+ <portNames>
464+ <portName>rmiport</portName>
465+ </portNames>
466+ </configuration>
467+ </execution>
468+ </executions>
469+ </plugin>
470+ <plugin>
471+ <groupId>org.apache.maven.plugins</groupId>
472+ <artifactId>maven-compiler-plugin</artifactId>
473+ <version>2.0.2</version>
474+ <configuration>
475+ <source>1.6</source>
476+ <target>1.6</target>
477+ </configuration>
478+ </plugin>
479+ <plugin>
480+ <groupId>org.apache.maven.plugins</groupId>
481+ <artifactId>maven-source-plugin</artifactId>
482+ <version>2.0.4</version>
483+ <executions>
484+ <execution>
485+ <id>attach-sources</id>
486+ <phase>package</phase>
487+ <goals>
488+ <goal>jar</goal>
489+ </goals>
490+ </execution>
491+ </executions>
492+ </plugin>
493+ <plugin>
494+ <groupId>org.apache.maven.plugins</groupId>
495+ <artifactId>maven-surefire-plugin</artifactId>
496+ <version>2.4.1</version>
497+ <configuration>
498+ <argLine>-Xmx640m -Xms128m -Drmiport=${rmiport} -Xrunjdwp:transport=dt_socket,address=8000,suspend=n,server=y</argLine>
499+ <includes>
500+ <include>**/*Test.java</include>
501+ <include>**/*Test?.java</include>
502+ </includes>
503+ </configuration>
504+ </plugin>
505+ <plugin>
506+ <groupId>org.codehaus.mojo</groupId>
507+ <artifactId>rmic-maven-plugin</artifactId>
508+ <version>1.0</version>
509+ <configuration>
510+ <outputDirectory>target/classes/</outputDirectory>
511+ </configuration>
512+ <executions>
513+ <execution>
514+ <id>rmi compilation</id>
515+ <goals>
516+ <goal>rmic</goal>
517+ </goals>
518+ </execution>
519+ </executions>
520+ </plugin>
521+ <plugin>
522+ <artifactId>maven-assembly-plugin</artifactId>
523+ <configuration>
524+ <descriptors>
525+ <descriptor>src/main/resources/assembly_descriptor.xml</descriptor>
526+ </descriptors>
527+ </configuration>
528+ <executions>
529+ <execution>
530+ <id>make-assembly</id> <!-- This is used for inheritance merges ?? -->
531+ <phase>install</phase> <!-- Append to the install phase -->
532+ <goals>
533+ <goal>single</goal>
534+ </goals>
535+ </execution>
536+ </executions>
537+ </plugin>
538+ <plugin>
539+ <groupId>org.codehaus.mojo</groupId>
540+ <artifactId>findbugs-maven-plugin</artifactId>
541+ <version>1.2</version>
542+ <configuration>
543+ <findbugsXmlOutput>true</findbugsXmlOutput>
544+ <findbugsXmlWithMessages>true</findbugsXmlWithMessages>
545+ <xmlOutput>true</xmlOutput>
546+ <excludeFilterFile>findbugs-exclude.xml</excludeFilterFile>
547+ </configuration>
548+ </plugin>
549+ <plugin>
550+ <groupId>com.atlassian.maven.plugins</groupId>
551+ <artifactId>maven-clover2-plugin</artifactId>
552+ <version>3.0.1</version>
553+ <configuration>
554+ <generateHistorical>true</generateHistorical>
555+ <historyDir>/clover/history</historyDir>
556+ <license>${clover.license}</license>
557+ <generateXml>true</generateXml>
558+ </configuration>
559+ </plugin>
560+ <plugin>
561 <inherited>false</inherited>
562 <groupId>com.mycila.maven-license-plugin</groupId>
563 <artifactId>maven-license-plugin</artifactId>
564 <version>1.10.b1</version>
565 <configuration>
566 <!-- Note plugin WARNs header wasn't specified but still works as desired (multi-module bug?) -->
567- <header>HEADER.txt</header>
568+ <header>src/etc/HEADER.txt</header>
569 <headerSections>
570 <headerSection>
571 <key>__YEAR_SECTION__</key>
572@@ -54,8 +213,8 @@
573 <exclude>bench/**</exclude>
574 <!-- Docs -->
575 <exclude>doc/**</exclude>
576- <exclude>core/src/main/resources/**</exclude>
577- <exclude>core/src/test/resources/**</exclude>
578+ <exclude>src/main/resources/**</exclude>
579+ <exclude>src/test/resources/**</exclude>
580 <!-- IDE files -->
581 <exclude>.idea/**</exclude>
582 <exclude>.settings/**</exclude>
583@@ -81,4 +240,18 @@
584 </plugin>
585 </plugins>
586 </build>
587+ <reporting>
588+ <plugins>
589+ <plugin>
590+ <groupId>org.apache.maven.plugins</groupId>
591+ <artifactId>maven-surefire-report-plugin</artifactId>
592+ <version>2.4.2</version>
593+ </plugin>
594+ <plugin>
595+ <groupId>org.apache.maven.plugins</groupId>
596+ <artifactId>maven-javadoc-plugin</artifactId>
597+ <version>2.8</version>
598+ </plugin>
599+ </plugins>
600+ </reporting>
601 </project>
602
603=== renamed directory 'core/src' => 'src'
604=== added directory 'src/etc'
605=== renamed file 'HEADER.txt' => 'src/etc/HEADER.txt'
606=== renamed file 'core/findbugs-exclude.xml' => 'src/etc/findbugs-exclude.xml'
607=== renamed file 'core/run_stress_tests.py' => 'src/etc/run_stress_tests.py'
608=== modified file 'src/main/java/com/persistit/Exchange.java'
609--- core/src/main/java/com/persistit/Exchange.java 2012-05-25 18:50:59 +0000
610+++ src/main/java/com/persistit/Exchange.java 2012-05-29 19:27:22 +0000
611@@ -1031,7 +1031,6 @@
612 *
613 * @return Encoded key location within the data page. The page itself is
614 * made valid in the level cache.
615- * @throws PMapException
616 */
617 private int search(Key key, boolean writer) throws PersistitException {
618 Buffer buffer = null;
619@@ -1099,7 +1098,6 @@
620 *
621 * @return Encoded key location within the level. The page itself is valid
622 * within the level cache.
623- * @throws PMapException
624 */
625 private int searchTree(Key key, int toLevel, boolean writer) throws PersistitException {
626 Buffer oldBuffer = null;
627@@ -1198,7 +1196,6 @@
628 * @param currentLevel
629 * current level in the tree
630 * @return Encoded key location within the page.
631- * @throws PMapException
632 */
633 private int searchLevel(Key key, boolean edge, long pageAddress, int currentLevel, boolean writer)
634 throws PersistitException {
635@@ -1323,15 +1320,9 @@
636 * uponError
637 */
638 boolean storeInternal(Key key, Value value, int level, int options) throws PersistitException {
639- if ((options & StoreOptions.FETCH) > 0 && (options & StoreOptions.MVCC) > 0) {
640- throw new IllegalArgumentException("Both fetch and MVCC not supported");
641- }
642
643 final boolean doMVCC = (options & StoreOptions.MVCC) > 0;
644- final boolean doAnyFetch = (options & StoreOptions.FETCH) > 0 || doMVCC;
645-
646- // spare used for fetch
647- Debug.$assert0.t(!doAnyFetch || value != _spareValue);
648+ final boolean doFetch = (options & StoreOptions.FETCH) > 0;
649
650 // spares used for new splits/levels
651 Debug.$assert0.t(key != _spareKey1);
652@@ -1344,6 +1335,8 @@
653 boolean incrementMVVCount = false;
654
655 final int maxSimpleValueSize = maxValueSize(key.getEncodedSize());
656+ final Value spareValue = _persistit.getThreadLocalValue();
657+ assert !(doMVCC & value == spareValue || doFetch && value == _spareValue): "storeInternal may be use the supplied Value: " + value;
658
659 //
660 // First insert the record in the data page
661@@ -1369,7 +1362,7 @@
662 // This method may delay significantly for I/O and must
663 // be called when there are no other claimed resources.
664 //
665- newLongRecordPointer = getLongRecordHelper().storeLongRecord(value, _transaction.isActive());
666+ newLongRecordPointer = getLongRecordHelper().storeLongRecord(value, _transaction.isActive());
667 }
668
669 if (!_ignoreTransactions && ((options & StoreOptions.DONT_JOURNAL) == 0)) {
670@@ -1396,7 +1389,7 @@
671 if (!committed && newLongRecordPointerMVV != 0) {
672 _volume.getStructure().deallocateGarbageChain(newLongRecordPointerMVV, 0);
673 newLongRecordPointerMVV = 0;
674- _spareValue.changeLongRecordMode(false);
675+ spareValue.changeLongRecordMode(false);
676 }
677
678 if (treeClaimRequired && !treeClaimAcquired) {
679@@ -1462,37 +1455,38 @@
680 if (keyExisted) {
681 oldLongRecordPointer = buffer.fetchLongRecordPointer(foundAt);
682 }
683- if (doAnyFetch) {
684- buffer.fetch(foundAt, _spareValue);
685- /*
686- * If we aren't in MVCC we have to un-long-ify as
687- * fetch was requested. Otherwise only do it if it
688- * is a long MVV so as to not-needlessly create one.
689- */
690- if (!doMVCC) {
691- fetchFixupForLongRecords(_spareValue, Integer.MAX_VALUE);
692- } else if (oldLongRecordPointer != 0) {
693- if (isLongMVV(_spareValue)) {
694+
695+ if (doFetch || doMVCC) {
696+ buffer.fetch(foundAt, spareValue);
697+ if (oldLongRecordPointer != 0) {
698+ if (isLongMVV(spareValue)) {
699 oldLongRecordPointerMVV = oldLongRecordPointer;
700- fetchFixupForLongRecords(_spareValue, Integer.MAX_VALUE);
701+ fetchFixupForLongRecords(spareValue, Integer.MAX_VALUE);
702 }
703- /*
704- * If it was a long MVV we saved it into the
705- * variable above. Otherwise it is a primordial
706- * value that we can't get rid of.
707- */
708- oldLongRecordPointer = 0;
709+ }
710+ /*
711+ * If it was a long MVV we saved it into the
712+ * variable above. Otherwise it is a
713+ * primordial value that we can't get rid
714+ * of.
715+ */
716+ oldLongRecordPointer = 0;
717+
718+ if (doFetch) {
719+ spareValue.copyTo(_spareValue);
720+ fetchFromValueInternal(_spareValue, Integer.MAX_VALUE, buffer);
721 }
722 }
723+
724 if (doMVCC) {
725- valueToStore = _spareValue;
726+ valueToStore = spareValue;
727 int valueSize = value.getEncodedSize();
728 /*
729 * If key didn't exist the value is truly
730 * non-existent and not just undefined/zero length
731 */
732- byte[] spareBytes = _spareValue.getEncodedBytes();
733- int spareSize = keyExisted ? _spareValue.getEncodedSize() : -1;
734+ byte[] spareBytes = spareValue.getEncodedBytes();
735+ int spareSize = keyExisted ? spareValue.getEncodedSize() : -1;
736 spareSize = MVV.prune(spareBytes, 0, spareSize, _persistit.getTransactionIndex(), false,
737 prunedVersions);
738
739@@ -1521,8 +1515,8 @@
740 MVV.visitAllVersions(_mvvVisitor, spareBytes, 0, spareSize);
741
742 int mvvSize = MVV.estimateRequiredLength(spareBytes, spareSize, valueSize);
743- _spareValue.ensureFit(mvvSize);
744- spareBytes = _spareValue.getEncodedBytes();
745+ spareValue.ensureFit(mvvSize);
746+ spareBytes = spareValue.getEncodedBytes();
747
748 long versionHandle = TransactionIndex.tss2vh(_transaction.getStartTimestamp(), tStep);
749 int storedLength = MVV.storeVersion(spareBytes, 0, spareSize, spareBytes.length,
750@@ -1530,11 +1524,10 @@
751
752 incrementMVVCount = (storedLength & MVV.STORE_EXISTED_MASK) == 0;
753 storedLength &= MVV.STORE_LENGTH_MASK;
754- _spareValue.setEncodedSize(storedLength);
755+ spareValue.setEncodedSize(storedLength);
756
757- if (_spareValue.getEncodedSize() > maxSimpleValueSize) {
758- newLongRecordPointerMVV = getLongRecordHelper().storeLongRecord(_spareValue,
759- _transaction.isActive());
760+ if (spareValue.getEncodedSize() > maxSimpleValueSize) {
761+ newLongRecordPointerMVV = getLongRecordHelper().storeLongRecord(spareValue, _transaction.isActive());
762 }
763 }
764 }
765@@ -1673,7 +1666,7 @@
766 }
767
768 value.changeLongRecordMode(false);
769- _spareValue.changeLongRecordMode(false);
770+ spareValue.changeLongRecordMode(false);
771 if (!committed) {
772 //
773 // We failed to write the new LONG_RECORD. If there was
774@@ -1698,7 +1691,7 @@
775 }
776 _volume.getStatistics().bumpStoreCounter();
777 _tree.getStatistics().bumpStoreCounter();
778- if (doAnyFetch) {
779+ if (doFetch || doMVCC) {
780 _volume.getStatistics().bumpFetchCounter();
781 _tree.getStatistics().bumpFetchCounter();
782 }
783@@ -1761,7 +1754,6 @@
784 * The encoded insert location.
785 * @return <code>true</code> if it necessary to insert a key into the
786 * ancestor index page.
787- * @throws PMapException
788 */
789 // TODO - Check insertIndexLevel timestamps
790 private boolean putLevel(LevelCache lc, Key key, ValueHelper valueWriter, Buffer buffer, int foundAt,
791@@ -2142,7 +2134,7 @@
792 index = _key.getEncodedSize();
793
794 if (matches) {
795- matches = fetchInternal(buffer, outValue, foundAt, minimumBytes);
796+ matches = fetchFromBufferInternal(buffer, outValue, foundAt, minimumBytes);
797 if (!matches && direction != EQ) {
798 nudged = false;
799 nudgeForMVCC = (direction == GTEQ || direction == LTEQ);
800@@ -2162,7 +2154,7 @@
801 if (matches) {
802 index = _key.nextElementIndex(parentIndex);
803 if (index > 0) {
804- boolean isVisibleMatch = fetchInternal(buffer, outValue, foundAt, minimumBytes);
805+ boolean isVisibleMatch = fetchFromBufferInternal(buffer, outValue, foundAt, minimumBytes);
806 //
807 // In any case (matching sibling, child or
808 // niece/nephew) we need to ignore this
809@@ -2569,7 +2561,9 @@
810 _persistit.checkClosed();
811 _persistit.checkSuspended();
812 _key.testValidForStoreAndFetch(_volume.getPageSize());
813- storeInternal(_key, _value, 0, StoreOptions.FETCH | StoreOptions.WAIT);
814+ int options = StoreOptions.WAIT | StoreOptions.FETCH;
815+ options |= (!_ignoreTransactions && _transaction.isActive()) ? StoreOptions.MVCC : 0;
816+ storeInternal(_key, _value, 0, options);
817 _spareValue.copyTo(_value);
818 return this;
819 }
820@@ -2727,7 +2721,7 @@
821 if (minimumBytes < 0) {
822 minimumBytes = 0;
823 }
824- fetchInternal(value, minimumBytes);
825+ searchAndFetchInternal(value, minimumBytes);
826 return this;
827 }
828
829@@ -2748,9 +2742,29 @@
830 * As thrown from any internal method.
831 * @return <code>true</code> if the value was visible.
832 */
833- private boolean fetchInternal(Buffer buffer, Value value, int foundAt, int minimumBytes) throws PersistitException {
834+ private boolean fetchFromBufferInternal(Buffer buffer, Value value, int foundAt, int minimumBytes) throws PersistitException {
835+ buffer.fetch(foundAt, value);
836+ return fetchFromValueInternal(value, minimumBytes, buffer);
837+ }
838+
839+ /**
840+ * Helper for finalizing the value to return from a, potentially, MVV
841+ * contained in the given Value.
842+ *
843+ * @param value
844+ * Value to finalize.
845+ * @param minimumBytes
846+ * Minimum amount of LONG_RECORD to fetch. If &lt;0, the
847+ * <code>value</code> will contain just the descriptor portion.
848+ * @param bufferForPruning
849+ * If not <code>null</code> and <code>Value</code> did contain
850+ * an MVV, call {@link Buffer#enqueuePruningAction(int)}.
851+ * @throws PersistitException
852+ * As thrown from any internal method.
853+ * @return <code>true</code> if the value was visible.
854+ */
855+ private boolean fetchFromValueInternal(Value value, int minimumBytes, Buffer bufferForPruning) throws PersistitException {
856 boolean visible = true;
857- buffer.fetch(foundAt, value);
858 /*
859 * We must fetch the full LONG_RECORD, if needed, while buffer is
860 * claimed from calling code so that it can't be de-allocated as we are
861@@ -2763,7 +2777,9 @@
862 */
863 fetchFixupForLongRecords(value, Integer.MAX_VALUE);
864 if (MVV.isArrayMVV(value.getEncodedBytes(), 0, value.getEncodedSize())) {
865- buffer.enqueuePruningAction(_tree.getHandle());
866+ if (bufferForPruning != null) {
867+ bufferForPruning.enqueuePruningAction(_tree.getHandle());
868+ }
869 visible = mvccFetch(value, minimumBytes);
870 fetchFixupForLongRecords(value, minimumBytes);
871 }
872@@ -2790,13 +2806,13 @@
873 * @throws PersistitException
874 * As thrown from {@link #search(Key, boolean)}
875 */
876- private void fetchInternal(Value value, int minimumBytes) throws PersistitException {
877+ private void searchAndFetchInternal(Value value, int minimumBytes) throws PersistitException {
878 Buffer buffer = null;
879 try {
880 int foundAt = search(_key, false);
881 LevelCache lc = _levelCache[0];
882 buffer = lc._buffer;
883- fetchInternal(buffer, value, foundAt, minimumBytes);
884+ fetchFromBufferInternal(buffer, value, foundAt, minimumBytes);
885 _volume.getStatistics().bumpFetchCounter();
886 _tree.getStatistics().bumpFetchCounter();
887 } finally {
888@@ -3063,7 +3079,7 @@
889
890 _value.clear().putAntiValueMVV();
891 final int storeOptions = StoreOptions.MVCC | StoreOptions.WAIT | StoreOptions.ONLY_IF_VISIBLE
892- | StoreOptions.DONT_JOURNAL;
893+ | StoreOptions.DONT_JOURNAL | (fetchFirst ? StoreOptions.FETCH : 0);
894
895 boolean anyRemoved = false;
896 boolean keyIsLessThan = true;
897@@ -3755,8 +3771,7 @@
898 /**
899 * Called by Transaction to set up a context for committing updates.
900 *
901- * @param volume
902- * @param _treeName
903+ * @param tree
904 */
905 void setTree(Tree tree) throws PersistitException {
906 _persistit.checkClosed();
907@@ -3944,7 +3959,7 @@
908 boolean savedIgnore = _ignoreMVCCFetch;
909 try {
910 _ignoreMVCCFetch = true;
911- fetchInternal(_spareValue, -1);
912+ searchAndFetchInternal(_spareValue, -1);
913 final boolean wasLong = isLongRecord(_spareValue);
914 _spareValue.clear();
915 return wasLong;
916@@ -3966,7 +3981,7 @@
917 boolean savedIgnore = _ignoreMVCCFetch;
918 try {
919 _ignoreMVCCFetch = true;
920- fetchInternal(_spareValue, -1);
921+ searchAndFetchInternal(_spareValue, -1);
922 final boolean wasLong = isLongMVV(_spareValue);
923 _spareValue.clear();
924 return wasLong;
925
926=== added file 'src/test/java/com/persistit/Bug912514Test.java'
927--- src/test/java/com/persistit/Bug912514Test.java 1970-01-01 00:00:00 +0000
928+++ src/test/java/com/persistit/Bug912514Test.java 2012-05-29 19:27:22 +0000
929@@ -0,0 +1,114 @@
930+/**
931+ * Copyright © 2012 Akiban Technologies, Inc. All rights reserved.
932+ *
933+ * This program is free software: you can redistribute it and/or modify
934+ * it under the terms of the GNU Affero General Public License as
935+ * published by the Free Software Foundation, version 3 (only) of the
936+ * License.
937+ *
938+ * This program is distributed in the hope that it will be useful,
939+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
940+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
941+ * GNU Affero General Public License for more details.
942+ *
943+ * You should have received a copy of the GNU Affero General Public License
944+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
945+ *
946+ * This program may also be available under different license terms. For more
947+ * information, see www.akiban.com or contact licensing@akiban.com.
948+ */
949+
950+package com.persistit;
951+
952+import static org.junit.Assert.*;
953+
954+import org.junit.Test;
955+
956+import com.persistit.unit.PersistitUnitTestCase;
957+
958+/**
959+ * https://bugs.launchpad.net/akiban-persistit/+bug/912514
960+ *
961+ * PersistitMapStress1 fails intermittently with messages like this:
962+ *
963+ * Finished unit=#1 PersistitMapStress test=PersistitMapStress1 main at ts=8036
964+ * - elapsed=5738 - FAILED: value not expected to be null 8036 Failed test
965+ * unit=#1 PersistitMapStress test=PersistitMapStress1 main value not expected
966+ * to be null
967+ *
968+ * I didn't track down the responsible code, but I'm pretty sure the contents of
969+ * _spareValue have been mangled.
970+ *
971+ * Since akiban-server does not use this method the bug is no more than medium
972+ * priority. But it does need to be fixed before akiban-persistit is released as
973+ * a standalone library.
974+ */
975+
976+public class Bug912514Test extends PersistitUnitTestCase {
977+
978+ private final static String ROLLBACK = "rollback";
979+
980+ private void fetchAndStoreAndRemoveHelper(final boolean inTxn, final String... sequence) throws Exception {
981+ final Transaction txn = _persistit.getTransaction();
982+ final Exchange exchange = _persistit.getExchange("persistit", "Bug912514Test", true);
983+ String previous = null;
984+ for (String string : sequence) {
985+
986+ if (inTxn) {
987+ txn.begin();
988+ }
989+
990+ if (string == null) {
991+ exchange.to(1).fetchAndRemove();
992+ } else {
993+ exchange.getValue().put(string);
994+ exchange.to(1).fetchAndStore();
995+ }
996+ compare(previous, exchange.getValue());
997+
998+ if (inTxn) {
999+ if (string.startsWith(ROLLBACK)) {
1000+ txn.rollback();
1001+ } else {
1002+ txn.commit();
1003+ }
1004+ txn.end();
1005+ compare(previous, exchange.getValue());
1006+ }
1007+
1008+ if (!inTxn || !string.startsWith(ROLLBACK)) {
1009+ previous = string;
1010+ }
1011+ exchange.fetch();
1012+ compare(previous, exchange.getValue());
1013+
1014+
1015+ }
1016+ }
1017+
1018+ private void compare(final String string, final Value value) {
1019+ if (string == null) {
1020+ assertTrue("Value should be undefined", !value.isDefined());
1021+ } else {
1022+ assertEquals("Value should match", string, value.getString());
1023+ }
1024+ }
1025+
1026+ @Test
1027+ public void fetchAndStoreTxn() throws Exception {
1028+ fetchAndStoreAndRemoveHelper(true, RED_FOX, createString(100), createString(1000), createString(10000), RED_FOX);
1029+ }
1030+
1031+ @Test
1032+ public void fetchAndRemoveNonTxn() throws Exception {
1033+ fetchAndStoreAndRemoveHelper(false, RED_FOX, null, null, createString(100), null, createString(1000), null,
1034+ createString(10000), null, null, RED_FOX, null);
1035+ }
1036+
1037+ @Test
1038+ public void fetchAndStoreTxnWithRollbacks() throws Exception {
1039+ fetchAndStoreAndRemoveHelper(true, RED_FOX, createString(100), ROLLBACK, createString(1000), ROLLBACK
1040+ + createString(10000), createString(10000), RED_FOX);
1041+ }
1042+
1043+}

Subscribers

People subscribed via source and target branches