HDDS-2325. BenchMarkDatanodeDispatcher genesis test is failing with NPE

[elek]

What changes were proposed in this pull request?

TLDR; Fix BenchmarkDatanodeDispatcher.writeChunk test.

Genesis is a microbenchmark tool for Ozone based on JMH (https://openjdk.java.net/projects/code-tools/jmh/).

Due to the recent Datanode changes the BenchMarkDatanodeDispatcher is failing with NPE:

java.lang.NullPointerException
	at org.apache.hadoop.ozone.container.common.interfaces.Handler.<init>(Handler.java:69)
	at org.apache.hadoop.ozone.container.keyvalue.KeyValueHandler.<init>(KeyValueHandler.java:114)
	at org.apache.hadoop.ozone.container.common.interfaces.Handler.getHandlerForContainerType(Handler.java:78)
	at org.apache.hadoop.ozone.genesis.BenchMarkDatanodeDispatcher.initialize(BenchMarkDatanodeDispatcher.java:115)
	at org.apache.hadoop.ozone.genesis.generated.BenchMarkDatanodeDispatcher_createContainer_jmhTest._jmh_tryInit_f_benchmarkdatanodedispatcher0_G(BenchMarkDatanodeDispatcher_createContainer_jmhTest.java:438)
	at org.apache.hadoop.ozone.genesis.generated.BenchMarkDatanodeDispatcher_createContainer_jmhTest.createContainer_Throughput(BenchMarkDatanodeDispatcher_createContainer_jmhTest.java:71)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.openjdk.jmh.runner.BenchmarkHandler$BenchmarkTask.call(BenchmarkHandler.java:453)
	at org.openjdk.jmh.runner.BenchmarkHandler$BenchmarkTask.call(BenchmarkHandler.java:437)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

And this is the just the biggest problem there are a few other problems. I propose the following fixes:

fix 1: NPE is thrown because the 'context' object is required by KeyValueHandler/Handler classes.

In fact the context is not required, we need two functionalities/info from the context: the ability to send icr (IncrementalContainerReport) and the ID of the datanode.

Law of Demeter principle suggests to have only the minimum required information from other classes.

For example instead of having context but using only context.getParent().getDatanodeDetails().getUuidString() we can have only the UUID string which makes more easy to test (unit and benchmark) the Handler/KeyValueHandler.

This is the biggest (but still small change) in this patch: I started to use the datanodeId and an icrSender instead of having the full context.

fix 2,3: There were a few other problems. The scmId was missing if the writeChunk was called from Benchmark and and the Checksum was also missing.

fix 4: I also had a few other problems: very huge containers are used (default 5G) and as the benchmark starts with creating 100 containers it requires 500G space by default. I adjusted the container size to make it possible to run on local machine.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-2325

How was this patch tested?

./ozone genesis -benchmark=BenchMarkDatanodeDispatcher.writeChunk

[elek]

notification test (notification is supposed to be sent to [email protected])

[adoroszlai]

Thanks @elek for fixing the test for writeChunk.

I agree that passing specific dependencies is cleaner. Context wouldn't be bad if it contained those exact dependencies instead of some "parent" etc.

Since this PR needs to be rebased for conflicts, please consider my nits below.

[adoroszlai]

Nit: odd spacing for no-op consumer (this one looks cleaner). Same in a few other places.

[elek]

Hard to avoid this as this is the default formatting rule of IntelliJ. Will be reformatted by the next auto-format anyway.

But I created a constant field, hopefully it makes it a little more readable:

  public static final Consumer<ContainerReplicaProto> NO_OP_ICR_SENDER = c -> {};

[arp7]

+1 the changes look good to me.

[elek]

Thanks the review @arp7 and @adoroszlai

I re-tested it locally and committed it to the master branch.