Make sorted recovery write to RFiles #2117

milleruntime · 2021-05-25T17:42:25Z

Change LogSorter and SortedLogRecovery to write/read RFile instead of MapFile
Rewrite LogSorter writeBuffer method and make static to test
Create toKey() and fromKey() methods in LogFileKey for converting WAL keys
Create toValue() and FromValue() methods in LogFileValue for converting WAL values
Byte encode the rows of the Key to sort WAL events properly
Make RecoveryLogsIterator convert Key Value pairs on next()

milleruntime · 2021-05-25T19:02:46Z

Ran full ITs and saw timeout failures in: CreateInitialSplitsIT, SuspendedTabletsIT

milleruntime · 2021-05-26T14:23:27Z

Ran full ITs and saw timeout failures in: CreateInitialSplitsIT, SuspendedTabletsIT

Both ITs passed when run locally.

server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java

server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogsIterator.java

milleruntime · 2021-05-26T15:57:20Z

I found a bug with the writeBuffer method while testing in Uno.

2021-05-26T11:00:44,630 [tserver.AssignmentHandler] WARN : exception trying to assign tablet +r<< null
java.lang.RuntimeException: Error recovering tablet +r<< from log files
        at org.apache.accumulo.tserver.tablet.Tablet.(Tablet.java:398) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.AssignmentHandler.run(AssignmentHandler.java:160) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at java.lang.Thread.run(Thread.java:829) [?:?]
Caused by: java.io.IOException: java.lang.IllegalStateException: First log entry value is not OPEN
        at org.apache.accumulo.tserver.log.TabletServerLogger.recover(TabletServerLogger.java:539) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.TabletServer.recover(TabletServer.java:1148) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.tablet.Tablet.(Tablet.java:357) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        ... 2 more
Caused by: java.lang.IllegalStateException: First log entry value is not OPEN
        at org.apache.accumulo.tserver.log.RecoveryLogsIterator.validateFirstKey(RecoveryLogsIterator.java:151) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.log.RecoveryLogsIterator.getFiles(RecoveryLogsIterator.java:133) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.log.RecoveryLogsIterator.(RecoveryLogsIterator.java:71) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.log.SortedLogRecovery.findMaxTabletId(SortedLogRecovery.java:113) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.log.SortedLogRecovery.findLogsThatDefineTablet(SortedLogRecovery.java:153) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.log.SortedLogRecovery.recover(SortedLogRecovery.java:299) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.log.TabletServerLogger.recover(TabletServerLogger.java:537) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.TabletServer.recover(TabletServer.java:1148) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        at org.apache.accumulo.tserver.tablet.Tablet.(Tablet.java:357) ~[accumulo-tserver-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
        ... 2 more

This is because the entries get flushed in the LogSorter based on tserver.sort.buffer.size but the validation is still being run for each sorted part. I will have to find a better place to call validateFirstKey.

milleruntime · 2021-05-26T19:56:56Z

Need to rework my rework in 007d581

milleruntime · 2021-06-02T19:34:04Z

I successfully tested the recovery locally using Uno with TestIngest of 50000 rows and using two tservers. Ran all the ITs again and WriteAheadLogEncryptedIT was the only one failing. I refactored to pass server context in order to get the configuration and it now passes.

There may be room for future performance improvements but I think the equivalent functionality of sorting recovery with RFile is ready to merge. This change should take care of #2076 as well but I want to do further testing with encryption since I don't think it works exactly how we want it to yet. I can do that as a follow on task.

keith-turner

I started reviewing this, but was not able to finish. I will come back to it later. Posting the comments I made so far.

server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java

keith-turner

Still looking at this, but I got farther.

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileValue.java

server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogsIterator.java

server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java

server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogsIterator.java

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java

milleruntime · 2021-06-15T14:20:21Z

@keith-turner Did you want to take another look at these changes? I think this is ready to merge

keith-turner · 2021-06-15T17:01:05Z

@keith-turner Did you want to take another look at these changes? I think this is ready to merge

currently looking at it

keith-turner

@milleruntime it tool a while but I finally finished reviewing the entire change. This is a really nice change. Even though the diff is small, its a complex change so it took time to review.

server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogsIterator.java

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java

server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogsIterator.java

Create LogFileKey.toKey() and LogFileValue.toValue() methods Make RecoveryLogsIterator iterate over Key Value Add fromKey and fromValue methods for parsing from Key Rewrite LogSorter writeBuffer method and make static to test

…r/LogFileKey.java Co-authored-by: Keith Turner <kturner@apache.org>

ctubbsii

Just a few minor notes from a quick pass

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileValue.java

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java

server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogsIterator.java

…r/LogFileValue.java Co-authored-by: Christopher Tubbs <ctubbsii@apache.org>

…r/LogFileKey.java Co-authored-by: Christopher Tubbs <ctubbsii@apache.org>

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java

* Add upgradeFiles method for upgrading files to Upgrader * Refactor status handling in UpgradeCoordinator to allow upgradeFiles to run concurrently and wait for metadata upgrade to complete * Implement upgradeFiles method in Upgrader9to10 * Add dropSortedMapWALFiles for resolving sorted map files that may still be around during upgrade. Follow up for apache#2117 * Closes apache#2179

* Add dropSortedMapWALFiles to Upgrader9to10 for resolving sorted map files that may still be around during upgrade. Follow up for #2117 * Closes #2179

DomGarguilo reviewed May 26, 2021

View reviewed changes

server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java Show resolved Hide resolved

DomGarguilo reviewed May 26, 2021

View reviewed changes

server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogsIterator.java Show resolved Hide resolved

DomGarguilo mentioned this pull request May 27, 2021

PopulateMetadataTable Improvements #2118

Merged

milleruntime force-pushed the sorted-rfile branch from f3bcb50 to 475d1ad Compare June 2, 2021 11:29

milleruntime mentioned this pull request Jun 2, 2021

Broken or Flaky tests after compaction changes: CompactionIT, KeywordStartIT, DeprecatedPropertyUpgrade9to10IT #2137

Closed

keith-turner reviewed Jun 4, 2021

View reviewed changes

keith-turner reviewed Jun 8, 2021

View reviewed changes

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java Show resolved Hide resolved

keith-turner reviewed Jun 15, 2021

View reviewed changes

ctubbsii added this to In progress in 2.1.0 via automation Jun 16, 2021

This was referenced Jun 16, 2021

Look into reusing open map files during log recovery. #882

Open

Add test and fix for bug in KeyBuilder #2170

Merged

milleruntime and others added 11 commits June 17, 2021 10:27

Change sorted WALs to write to RFiles

8a52be2

Create LogFileKey.toKey() and LogFileValue.toValue() methods Make RecoveryLogsIterator iterate over Key Value Add fromKey and fromValue methods for parsing from Key Rewrite LogSorter writeBuffer method and make static to test

Improve writeBuffer method

5d1920a

Revert timeout increase

e546835

Add log path to exception

aad476a

Rework RecoveryLogsIterator and SortedLogRecoveryTest for multipart logs

a1ba8f3

Only check first key once, before setting scan range

b69ebee

Clean up SortedLogRecoveryTest

42410a5

Create scanner per WAL part for better memory usage

84785a6

Make logging consistent with old recovery

ef1d2c5

Refactor to pass server context to provide configuration

91fbd0b

Update server/tserver/src/main/java/org/apache/accumulo/tserver/logge…

4afab5e

…r/LogFileKey.java Co-authored-by: Keith Turner <kturner@apache.org>

milleruntime and others added 8 commits June 17, 2021 10:27

Remove setting extent in min and max keys

d688522

Some refactoring from PR

ff53d8a

Update server/tserver/src/main/java/org/apache/accumulo/tserver/logge…

0609f37

…r/LogFileKey.java Co-authored-by: Keith Turner <kturner@apache.org>

Update server/tserver/src/main/java/org/apache/accumulo/tserver/logge…

05014a3

…r/LogFileKey.java Co-authored-by: Keith Turner <kturner@apache.org>

Update server/tserver/src/main/java/org/apache/accumulo/tserver/logge…

f47faec

…r/LogFileKey.java Co-authored-by: Keith Turner <kturner@apache.org>

Update server/tserver/src/main/java/org/apache/accumulo/tserver/logge…

2f40e3e

…r/LogFileKey.java Co-authored-by: Keith Turner <kturner@apache.org>

Update server/tserver/src/main/java/org/apache/accumulo/tserver/logge…

7a147ae

…r/LogFileKey.java Co-authored-by: Keith Turner <kturner@apache.org>

Drop colFamToFetch

e831472

milleruntime force-pushed the sorted-rfile branch from ee422b7 to e831472 Compare June 17, 2021 14:41

keith-turner approved these changes Jun 17, 2021

View reviewed changes

ctubbsii reviewed Jun 17, 2021

View reviewed changes

milleruntime and others added 3 commits June 17, 2021 12:13

Update server/tserver/src/main/java/org/apache/accumulo/tserver/logge…

352af97

…r/LogFileValue.java Co-authored-by: Christopher Tubbs <ctubbsii@apache.org>

Update server/tserver/src/main/java/org/apache/accumulo/tserver/logge…

6cd0ef8

…r/LogFileKey.java Co-authored-by: Christopher Tubbs <ctubbsii@apache.org>

CR updates

6fc3582

ctubbsii approved these changes Jun 18, 2021

View reviewed changes

milleruntime changed the title ~~Change sorted recovery to write to RFiles~~ Make sorted recovery write to RFiles Jun 21, 2021

milleruntime merged commit 3e765b1 into apache:main Jun 21, 2021

2.1.0 automation moved this from In progress to Done Jun 21, 2021

milleruntime deleted the sorted-rfile branch June 21, 2021 13:53

This was referenced Jun 22, 2021

Update the LogReader utility #2172

Closed

VerifySerialRecoveryIT failure #2173

Closed

ctubbsii reviewed Jun 22, 2021

View reviewed changes

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java Show resolved Hide resolved

ctubbsii reviewed Jun 22, 2021

View reviewed changes

server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java Show resolved Hide resolved

milleruntime mentioned this pull request Jun 24, 2021

Handle old sorted map files #2179

Closed

This was referenced Jun 30, 2021

Handle old sorted map files in Upgrade #2185

Merged

Sorted Log Recovery configuration design #2187

Closed

milleruntime added a commit that referenced this pull request Jul 30, 2021

Handle old sorted map files in Upgrade (#2185)

09e3d77

* Add dropSortedMapWALFiles to Upgrader9to10 for resolving sorted map files that may still be around during upgrade. Follow up for #2117 * Closes #2179

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sorted recovery write to RFiles #2117

Make sorted recovery write to RFiles #2117

milleruntime commented May 25, 2021 •

edited

milleruntime commented May 25, 2021

milleruntime commented May 26, 2021

milleruntime commented May 26, 2021

milleruntime commented May 26, 2021

milleruntime commented Jun 2, 2021

keith-turner left a comment

keith-turner left a comment

milleruntime commented Jun 15, 2021

keith-turner commented Jun 15, 2021

keith-turner left a comment

ctubbsii left a comment

Make sorted recovery write to RFiles #2117

Make sorted recovery write to RFiles #2117

Conversation

milleruntime commented May 25, 2021 • edited

milleruntime commented May 25, 2021

milleruntime commented May 26, 2021

milleruntime commented May 26, 2021

milleruntime commented May 26, 2021

milleruntime commented Jun 2, 2021

keith-turner left a comment

Choose a reason for hiding this comment

keith-turner left a comment

Choose a reason for hiding this comment

milleruntime commented Jun 15, 2021

keith-turner commented Jun 15, 2021

keith-turner left a comment

Choose a reason for hiding this comment

ctubbsii left a comment

Choose a reason for hiding this comment

milleruntime commented May 25, 2021 •

edited