Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8804 )
Change subject: KUDU-1704: add READ_OWN_WRITES scan mode ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/8804/1/src/kudu/common/common.proto File src/kudu/common/common.proto: http://gerrit.cloudera.org:8080/#/c/8804/1/src/kudu/common/common.proto@221 PS1, Line 221: : // When BOUNDED_READ is specified, the server will pick an arbitrarily snapshot : // in the past, subject to the propagated timestamp bound, and perform a read. : // More specifically, the server will choose the newest timestamp within the : // staleness bound that allows execution of the reads without blocking by the : // in-flight transactions. If no propagated timestamp is provided the serv > Not a big fan of BOUNDED_READ (bounded by what?) but I've had trouble to co I would also prefer READ_OWN_WRITES than READ_BOUNDED_STALE as the later does not explain much by the name. http://gerrit.cloudera.org:8080/#/c/8804/1/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/8804/1/src/kudu/tserver/tablet_service.cc@2036 PS1, Line 2036: switch (scan_pb.read_mode()) { : case READ_AT_SNAPSHOT: : case READ_OWN_WRITES: : > nit: use switch case Done http://gerrit.cloudera.org:8080/#/c/8804/1/src/kudu/tserver/tablet_service.cc@2041 PS1, Line 2041: return Status::NotSupported("Unsupported snapshot scan mode specified"); : } : : // Based on the read mode, pick a timestamp and verify it. : Timestamp tmp_snap_timestamp; : RETURN_NOT_OK(PickAndVerifyTimeStamp(scan_pb, tablet_replica, &tmp_snap_timestamp)); : : tablet::MvccSnapshot snap; : Tablet* tablet = tablet_replica->tablet(); : scoped_refptr<consensus::TimeManager> time_manager = tablet_replica->time_manager(); : tablet::MvccManager* mvcc_manager = tablet->mvcc_manager(); : : // Reduce the client's deadline by a few msecs to allow for overhead. : MonoTime client_deadline = rpc_context->GetClientDeadline() - MonoDelta::FromMilliseconds(10); : : // Its not good for the tablet server or for the client if we hang here forever. The tablet : // server will have one less available thread and the client might be stuck spending all : // of the allotted time for the scan on a partitioned server that will never have a consistent : // snapshot at 'snap_timestamp'. : // Because of this we clamp the client's deadline to the max. configured. If the client : // sets a long timeout then it can use it by trying in other servers. : bool was_clamped = false; : MonoTime final_deadline = ClampScanDeadlineForWait(client_deadline, &was_clamped); : : // Wait for the tablet to know that 'snap_timestamp' is safe. I.e. that all operations : // that came before it are, at least, started. This, together with waiting for the mvcc : // snapshot to be clean below, allows us to always return the same data when scanning at : // the same timestamp (repeatable reads). : TRACE("Waiting safe time to advance"); : MonoTime before = MonoTime::Now(); : Status s = time_manager->WaitUntilSafe(tmp_snap_timestamp, final_deadline); : : if (s.ok()) { : // Wait for the in-flights in the snapshot to be finished. : TRACE("Waiting for operations to commit"); : s = mvcc_manager->WaitForSnapshotWithAllCommitted(tmp_snap_timestamp, &snap, client_deadline); : } : : // If we got an TimeOut but we had clamped the deadline, return a ServiceUnavailable instead : // so that the client retries. : if (s.IsTimedOut() && was_clamped) { : return Status::ServiceUnavailable(s.CloneAndPrepend( : "could not wait for desired snapshot timestamp to be consistent").ToString()); : } : RETURN_NOT_OK(s); : : uint64_t duration_usec = (MonoTime::Now() - before).ToMicroseconds(); : tablet->metrics()->snapshot_read_inflight_wait_duration->Increment(duration_usec); : TRACE("All operations in snapshot committed. Waited for $0 microseconds", duration_usec); : > Can you move all of this to a new method? maybe PickOrValidateTimestamp() Done -- To view, visit http://gerrit.cloudera.org:8080/8804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48 Gerrit-Change-Number: 8804 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 13 Dec 2017 00:37:34 +0000 Gerrit-HasComments: Yes
