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

unit-PageTest regression due to commit 58445038 #2187

Closed
Aleaxander opened this issue Mar 28, 2014 · 9 comments
Closed

unit-PageTest regression due to commit 58445038 #2187

Aleaxander opened this issue Mar 28, 2014 · 9 comments
Assignees
Labels
Milestone

Comments

@Aleaxander
Copy link
Contributor

Hi @Tryneus,

JFYI, my test framework caught a unit-PageTest regression due to commit 5844503(" moving transaction throttling limit changing logic into the throttler").

I'm sorry for the noisy if you are already aware of this.

Here is the test log:

Running 1 of 178 tests (tasks: 1, repeats: 1, output dir: test_results.mA1nU, build: release, filter: unit-PageTest)
Run  unit-PageTest (1)
Note: Google Test filter = PageTest.*
[==========] Running 15 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 15 tests from PageTest
[ RUN      ] PageTest.Control
[       OK ] PageTest.Control (3 ms)
[ RUN      ] PageTest.CreateDestroy
[       OK ] PageTest.CreateDestroy (1 ms)
[ RUN      ] PageTest.OneTxn
[       OK ] PageTest.OneTxn (2 ms)
[ RUN      ] PageTest.TwoIndependentTxn
[       OK ] PageTest.TwoIndependentTxn (1 ms)
[ RUN      ] PageTest.TwoIndependentTxnSwitch
[       OK ] PageTest.TwoIndependentTxnSwitch (2 ms)
[ RUN      ] PageTest.TwoSequentialTxnSwitch
[       OK ] PageTest.TwoSequentialTxnSwitch (1 ms)
[ RUN      ] PageTest.OneWriteAcq
[       OK ] PageTest.OneWriteAcq (3 ms)
[ RUN      ] PageTest.OneWriteAcqOneReadAcq
[       OK ] PageTest.OneWriteAcqOneReadAcq (2 ms)
[ RUN      ] PageTest.OneWriteAcqWait
[       OK ] PageTest.OneWriteAcqWait (2 ms)
[ RUN      ] PageTest.ReadAfterWrite
[       OK ] PageTest.ReadAfterWrite (2 ms)
[ RUN      ] PageTest.WriteWaitForFlush
[       OK ] PageTest.WriteWaitForFlush (2 ms)
[ RUN      ] PageTest.BiggerTest
[       OK ] PageTest.BiggerTest (4 ms)
[ RUN      ] PageTest.BiggerTestTightMemory
Time unit-PageTest (1)
@Tryneus
Copy link
Member

Tryneus commented Mar 28, 2014

Thanks, @Aleaxander! Looking into this.

@Tryneus
Copy link
Member

Tryneus commented Mar 28, 2014

For the record, it appears that the unittests don't work with a transaction limit of 1, but I'm not sure if that holds for the server.

@Tryneus
Copy link
Member

Tryneus commented Mar 28, 2014

Just ran some tests on the server with --cache-size 0, and it appears this doesn't cause hangs in normal operation. I ran an insert workload across multiple clients while throwing in some sindex post-construction. Performance dipped fairly quickly (because with no cache, the throttler only allows one write through at a time), but no failures or hangs. The only slightly perturbing behavior was that sindex creation didn't finish until I stopped the write workload for a couple seconds, but that isn't exactly unexpected.

The PageTest unittests appear to make some assumptions about cache throttling that are no longer true (assumptions that the rest of the code doesn't make), so I think the test needs to be updated.

@AtnNn
Copy link
Member

AtnNn commented Mar 28, 2014

@Tryneus thanks for testing this. I will ignore that test for now.

@anatol
Copy link
Contributor

anatol commented Mar 28, 2014

Confirm that these 3 tests cause hang: PageTest.BiggerTestTightMemory,PageTest.BiggerTestSuperTightMemory,PageTest.BiggerTestNoMemory

I had to disable them in Linux Arch rethinkdb package.

@srh srh added this to the 1.12.x milestone Mar 28, 2014
@srh srh added the tp:bug label Mar 28, 2014
@srh srh self-assigned this Mar 28, 2014
@srh
Copy link
Contributor

srh commented Mar 28, 2014

The PageTest assumes that its write transactions are not going to get throttled. I might add a flag to the page_txn_t constructor that lets it force-acquire the throttling semaphore (which would either (a) also cause preceding transactions to force-acquire the throttling semaphore, or (b) cause preceding transactions on the same cache_conn_t to force-acquire the throttling semaphore). Or, I might decouple the throttler from the page_cache_t a bit.

@srh
Copy link
Contributor

srh commented Mar 28, 2014

Fixed in sam_2187 (branched off v1.12.x), awaiting code review 1384. I merely made the minimum unwritten changes limit be parameterizable.

@Aleaxander
Copy link
Contributor Author

@srh, yes, it does work.

@srh
Copy link
Contributor

srh commented Mar 29, 2014

In v1.12.x as of 83b85f0.

@srh srh closed this as completed Mar 29, 2014
@AtnNn AtnNn modified the milestones: 1.12.2, 1.12.x Apr 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants