Fix windows build and CI (facebook#12426)

Summary:
Issue facebook#12421 describes a regression in the migration from CircleCI to GitHub Actions in which failing build steps no longer fail Windows CI jobs. In GHA with pwsh (new preferred powershell command), only the last non-builtin command (or something like that) affects the overall success/failure result, and failures in external commands do not exit the script, even with `$ErrorActionPreference = 'Stop'` and `$PSNativeCommandErrorActionPreference = $true`. Switching to `powershell` causes some obscure failure (not seen in CircleCI) about the `-Lo` option to `curl`.

Here we work around this using the only reasonable-but-ugly way known: explicitly check the result after every non-trivial build step. This leaves us highly susceptible to future regressions with unchecked build steps in the future, but a clean solution is not known.

This change also fixes the build errors that were allowed to creep in because of the CI regression. Also decreased the unnecessarily long running time of DBWriteTest.WriteThreadWaitNanosCounter.

For background, this problem explicitly contradicts GitHub's documentation, and GitHub has known about the problem for more than a year, with no evidence of caring or intending to fix. actions/runner-images#6668 Somehow CircleCI doesn't have this problem. And even though cmd.exe and powershell have been perpetuating DOS-isms for decades, they still seem to be a somewhat active "hot mess" when it comes to sensible, consistent, and documented behavior.

Fixes facebook#12421

A history of some things I tried in development is here: facebook/rocksdb@main...pdillinger:rocksdb:debug_windows_ci_orig

Pull Request resolved: facebook#12426

Test Plan: CI, including facebook#12434 where I have temporarily enabled other Windows builds on PR with this change

Reviewed By: cbi42

Differential Revision: D54903698

Pulled By: pdillinger

fbshipit-source-id: 116bcbebbbf98f347c7cf7dfdeebeaaed7f76827

.github/actions/windows-build-steps/action.yml

@@ -15,30 +15,40 @@ runs:
       SNAPPY_INCLUDE: ${{ github.workspace }}/thirdparty/snappy-1.1.8;${{ github.workspace }}/thirdparty/snappy-1.1.8/build
       SNAPPY_LIB_DEBUG: ${{ github.workspace }}/thirdparty/snappy-1.1.8/build/Debug/snappy.lib
     run: |-
+      # NOTE: if ... Exit $LASTEXITCODE lines needed to exit and report failure
       echo ===================== Install Dependencies =====================
       choco install liberica8jdk -y
+      if(!$?) { Exit $LASTEXITCODE }
       mkdir $Env:THIRDPARTY_HOME
       cd $Env:THIRDPARTY_HOME
       echo "Building Snappy dependency..."
       curl -Lo snappy-1.1.8.zip https://github.com/google/snappy/archive/refs/tags/1.1.8.zip
+      if(!$?) { Exit $LASTEXITCODE }
       unzip -q snappy-1.1.8.zip
+      if(!$?) { Exit $LASTEXITCODE }
       cd snappy-1.1.8
       mkdir build
       cd build
       & cmake -G "$Env:CMAKE_GENERATOR" ..
+      if(!$?) { Exit $LASTEXITCODE }
       msbuild Snappy.sln -maxCpuCount -property:Configuration=Debug -property:Platform=x64
+      if(!$?) { Exit $LASTEXITCODE }
       echo ======================== Build RocksDB =========================
       cd ${{ github.workspace }}
       $env:Path = $env:JAVA_HOME + ";" + $env:Path
       mkdir build
       cd build
       & cmake -G "$Env:CMAKE_GENERATOR" -DCMAKE_BUILD_TYPE=Debug -DOPTDBG=1 -DPORTABLE="$Env:CMAKE_PORTABLE" -DSNAPPY=1 -DJNI=1 ..
+      if(!$?) { Exit $LASTEXITCODE }
       cd ..
       echo "Building with VS version: $Env:CMAKE_GENERATOR"
       msbuild build/rocksdb.sln -maxCpuCount -property:Configuration=Debug -property:Platform=x64
+      if(!$?) { Exit $LASTEXITCODE }
       echo ========================= Test RocksDB =========================
       build_tools\run_ci_db_test.ps1 -SuiteRun arena_test,db_basic_test,db_test,db_test2,db_merge_operand_test,bloom_test,c_test,coding_test,crc32c_test,dynamic_bloom_test,env_basic_test,env_test,hash_test,random_test -Concurrency 16
+      if(!$?) { Exit $LASTEXITCODE }
       echo ======================== Test RocksJava ========================
       cd build\java
       & ctest -C Debug -j 16
+      if(!$?) { Exit $LASTEXITCODE }
     shell: pwsh

db/c_test.c

@@ -55,10 +55,10 @@ static const char* GetTempDir(void) {
     ret = getenv("TEMP");
 #else
     ret = "/tmp";
-  }
 #endif
-  return ret;
   }
+  return ret;
+}
 #ifdef _MSC_VER
 #pragma warning(pop)
 #endif

db/db_write_test.cc

@@ -281,12 +281,12 @@ TEST_P(DBWriteTest, WriteThreadWaitNanosCounter) {
     perf_ctx->Reset();
     TEST_SYNC_POINT("DBWriteTest::WriteThreadWaitNanosCounter:WriteFunc");
     ASSERT_OK(dbfull()->Put(WriteOptions(), "bar", "val2"));
-    ASSERT_GT(perf_ctx->write_thread_wait_nanos, 1000000000U);
+    ASSERT_GT(perf_ctx->write_thread_wait_nanos, 2000000U);
   };
 
   std::function<void()> sleep_func = [&]() {
     TEST_SYNC_POINT("DBWriteTest::WriteThreadWaitNanosCounter:SleepFunc:1");
-    sleep(2);
+    SystemClock::Default()->SleepForMicroseconds(2000);
     TEST_SYNC_POINT("DBWriteTest::WriteThreadWaitNanosCounter:SleepFunc:2");
   };
 

db/external_sst_file_basic_test.cc

@@ -1958,7 +1958,8 @@ TEST_F(ExternalSSTFileBasicTest, IngestWithTemperature) {
     arg.options = in_opts;
     arg.files_checksums = files_checksums;
     arg.files_checksum_func_names = files_checksum_func_names;
-    if ((alternate_hint = !alternate_hint)) {
+    alternate_hint = !alternate_hint;
+    if (alternate_hint) {
       // Provide correct hint (for optimal file open performance)
       arg.file_temperature = Temperature::kWarm;
     } else {