Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

finatra: Remove setting flag value #7943

Merged
merged 1 commit into from
Feb 22, 2023
Merged

finatra: Remove setting flag value #7943

merged 1 commit into from
Feb 22, 2023

Conversation

cacoco
Copy link
Contributor

@cacoco cacoco commented Feb 22, 2023

Problem

In PR #7928 it was pointed out that the Finatra benchmark had been failing, due to a
failure of the server startup. Example logs here.

A change was made in Finatra in the middle of 2022 to remove a deprecated logging
shim which provided JUL logging configuration via command line flags. The consequence is
that setting this flag will now cause the server to fail to start (since the flag is no longer
defined anywhere in the server).

Solution

The solution is to remove the setting of this flag via the cmd line. This flag did not do anything
previously and should not be set. Having it in the benchmark was in error.

Result

The Finatra server under test should now start.

Testing

Tested via the tfb script:

]% ./tfb --mode verify --test finatra
...
finatra: [success] Total time: 78 s, completed Feb 22, 2023 4:46:43 PM
finatra: Removing intermediate container 41dc2c73e1c9
finatra:  ---> fe88c6d249b6
finatra: Step 7/8 : EXPOSE 8888
finatra:  ---> Running in 99cbe26898d0
finatra: Removing intermediate container 99cbe26898d0
finatra:  ---> 88501aa8a8e5
finatra: Step 8/8 : CMD ["java", "-Dio.netty.recycler.maxCapacityPerThread=0", "-Dio.netty.leakDetection.level=disabled", "-Dcom.twitter.finagle.tracing.enabled=false", "-server", "-XX:+UseNUMA", "-XX:+UseParallelGC", "-XX:+AggressiveOpts", "-jar", "target/scala-2.12/finatra-benchmark.jar", "-http.response.charset.enabled=false"]
finatra:  ---> Running in adb5247fb4db
finatra: Removing intermediate container adb5247fb4db
finatra:  ---> 932470edaa34
finatra: [Warning] One or more build-args [BENCHMARK_ENV TFB_TEST_DATABASE TFB_TEST_NAME] were not consumed
finatra: Successfully built 932470edaa34
finatra: Successfully tagged techempower/tfb.test.finatra:latest
finatra: Build time: 2m 58s
finatra: Verifying framework URLs
--------------------------------------------------------------------------------
VERIFYING JSON
--------------------------------------------------------------------------------
Accessing URL http://tfb-server:8888/json:
Accessing URL http://tfb-server:8888/json:
   PASS for http://tfb-server:8888/json
--------------------------------------------------------------------------------
VERIFYING PLAINTEXT
--------------------------------------------------------------------------------
Accessing URL http://tfb-server:8888/plaintext:
Accessing URL http://tfb-server:8888/plaintext:
   PASS for http://tfb-server:8888/plaintext
Auditing /FrameworkBenchmarks/frameworks/Scala/finatra:
No problems to report
wrk: Build time: 1m 3s
finatra: Build time: 2m 58s
finatra: Time starting database: 0s
finatra: Time until accepting requests: 6s
finatra: Verify time: 8s
finatra: Total test time: 3m 19s
tfb: Total time building so far: 4m 1s
tfb: Total time verifying so far: 8s
tfb: Total execution time so far: 4m 26s
================================================================================
Verification Summary
--------------------------------------------------------------------------------
| finatra
|       plaintext     : PASS
|       json          : PASS
================================================================================

Results are saved in /FrameworkBenchmarks/results/20230222164249

@NateBrady23 NateBrady23 merged commit cd712f9 into TechEmpower:master Feb 22, 2023
@cacoco cacoco deleted the cacoco/FixFinatra branch February 22, 2023 18:14
franz1981 pushed a commit to franz1981/FrameworkBenchmarks that referenced this pull request Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants