[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-20 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-845560449 Thanks. Unless objection I will merge this tonight or tomorrow morning, to master, and then to branch-2 (for future 2.5.0) -- This is an automated message from the Apache Git

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-20 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-845420407 Pushed a fix for whitespace and javadoc issues introduced in last change. No additional changes anticipated from this point. -- This is an automated message from the Apache

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-20 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-845417490 @ndimiduk > That failure looks suspicious. Let me grab the logs before you merge/close the PR. How can it be related? That test neither enables WAL compression nor

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-20 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-845354766 Unit test failures are not related. It looks like master is recently unstable, related to RS groups. -- This is an automated message from the Apache Git Service. To respond

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-19 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-844624849 SNAPPY or ZSTD are recommended, all other options provided for comparison. (LZMA is included as a sanity check that indeed an expensive algorithm really is expensive.)

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-19 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-844561641 I am redoing microbenchmarks with the latest patch and will update here soon. Removing the extra copy unlocked IO performance improvement from the compression.

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-19 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-844459128 @bharathv I wrote a simple bounded delegating input stream impl to avoid the unnecessary copy at decompression time. Rebased on master. Let me collect updated

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-19 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-844355147 > just extend FIS and add setDelegate (that makes it small). Oh, I misunderstood. Sure, I can do that. -- This is an automated message from the Apache Git Service. To

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-19 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-844301569 Unit test failure is not related. TestJMXListener.setupBeforeClass:68 » IO Shutting down Going to merge today unless objection. -- This is an automated

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-18 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-843686132 Rebased. Addressed last round of review feedback. Added another unit test. The split and replay tests which enable value compression already provide similar

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-14 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-841546694 Pushed a rebase on master and a fix for the latest findbugs warning. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-14 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-841533295 > Thanks for the detailed perf results, SNAPPY seems like the sweet spot. Avg append time took a good hit? (10x IIUC?), something to be concerned about? The extra latency

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-13 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-840922267 **Microbrenchmark Results** Site configuration used: hbase.master.logcleaner.ttl 60480

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-12 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-840250363 I took another bite at the @bharathv suggestion to use a streams API instead of driving Deflater and Inflater at the low level. A really excellent additional benefit, if this

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-12 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-839941587 Latest push addresses findbugs warnings. -- 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

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-11 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-838973994 > Speaking out loud. During RU, the wal written by RS that has ENABLE_WAL_VALUE_COMPRESSION set to false died and another RS which was recently upgraded has the above property

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-11 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-838765223 There was a period of time where there was a bad patch. I pushed a replacement. The latest precommits look good. -- This is an automated message from the Apache Git Service.

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-10 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-837688241 Rolled up the first round of review feedback into d4d707d -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-10 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-837666043 I have some changes coming soon. Accepted @bharathv 's argument that unconditionally compressing values if value compression is enabled is fine even if some value cases may not

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-10 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-837518016 > Is it reasonable to always use the same compressor all the time for WALs? It strikes me that operators make an explicit choice about the compression used in their tables, so

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-10 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-837512098 bq. There is a potential performance improvement here. We could create a Deflater/Inflater pair per column family. This idea didn't pan out. It makes compression in my

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-10 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-837063440 There is a potential performance improvement here. We could create a Deflater/Inflater pair per column family. The universe of column families across all schema in a production

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-10 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-837051331 **WAL Compression Results** Site configuration: hbase.master.logcleaner.ttl 60480

[GitHub] [hbase] apurtell commented on pull request #3244: HBASE-25869 WAL value compression

2021-05-10 Thread GitBox
apurtell commented on pull request #3244: URL: https://github.com/apache/hbase/pull/3244#issuecomment-836903862 > So we will only compress value? This is an enhancement to existing WAL compression. As you know the existing WAL compression already compresses other aspects of WAL