From 99ad42ab82ec5cafa5aba9d2d81dc32521ab32d7 Mon Sep 17 00:00:00 2001 From: dimitr Date: Sun, 24 Jan 2016 21:14:18 +0000 Subject: [PATCH] This should fix the races while accessing the relation statistics inside dbb_stats. Reads/writes/fetches/marks are incremented (CCH) and read (INF) unprotected, as it's crash safe and read skews are acceptable. Other counters (both global and relation wise) are protected with a mutex. They're incremented on demand based on the attachment counters (via diffs). While being there, cleaned up the currently unused performance counters. --- src/jrd/Attachment.cpp | 9 +++++++++ src/jrd/Attachment.h | 3 +++ src/jrd/Database.h | 3 ++- src/jrd/JrdStatement.cpp | 2 -- src/jrd/Monitoring.cpp | 6 +++++- src/jrd/Monitoring.h | 2 +- src/jrd/RuntimeStatistics.h | 21 +++++++++------------ src/jrd/cch.cpp | 2 -- src/jrd/exe.cpp | 3 --- src/jrd/jrd.cpp | 6 ++---- src/jrd/jrd.h | 28 +++++++++++++++++++++++----- src/jrd/sort.cpp | 6 ------ 12 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/jrd/Attachment.cpp b/src/jrd/Attachment.cpp index f5a4a17f66..41bf22a59e 100644 --- a/src/jrd/Attachment.cpp +++ b/src/jrd/Attachment.cpp @@ -179,6 +179,7 @@ Jrd::Attachment::Attachment(MemoryPool* pool, Database* dbb) att_lock_owner_id(Database::getLockOwnerId()), att_backup_state_counter(0), att_stats(*pool), + att_base_stats(*pool), att_working_directory(*pool), att_filename(*pool), att_timestamp(Firebird::TimeStamp::getCurrentTimeStamp()), @@ -381,6 +382,14 @@ void Jrd::Attachment::signalShutdown() } +void Jrd::Attachment::mergeStats() +{ + MutexLockGuard guard(att_database->dbb_stats_mutex, FB_FUNCTION); + att_database->dbb_stats.adjust(att_base_stats, att_stats, true); + att_base_stats.assign(att_stats); +} + + // Find an inactive incarnation of a system request. If necessary, clone it. jrd_req* Jrd::Attachment::findSystemRequest(thread_db* tdbb, USHORT id, USHORT which) { diff --git a/src/jrd/Attachment.h b/src/jrd/Attachment.h index bc8565a0e4..815a70fd9b 100644 --- a/src/jrd/Attachment.h +++ b/src/jrd/Attachment.h @@ -250,6 +250,7 @@ public: SecurityClass* att_security_class; // security class for database SecurityClassList* att_security_classes; // security classes RuntimeStatistics att_stats; + RuntimeStatistics att_base_stats; ULONG att_flags; // Flags describing the state of the attachment SSHORT att_client_charset; // user's charset specified in dpb SSHORT att_charset; // current (client or external) attachment charset @@ -355,6 +356,8 @@ public: void signalCancel(); void signalShutdown(); + void mergeStats(); + bool backupStateWriteLock(thread_db* tdbb, SSHORT wait); void backupStateWriteUnLock(thread_db* tdbb); bool backupStateReadLock(thread_db* tdbb, SSHORT wait); diff --git a/src/jrd/Database.h b/src/jrd/Database.h index 1ace58e59c..faba467139 100644 --- a/src/jrd/Database.h +++ b/src/jrd/Database.h @@ -447,8 +447,9 @@ public: Firebird::Semaphore dbb_gc_fini; // Event for finalization garbage collector Firebird::MemoryStats dbb_memory_stats; - RuntimeStatistics dbb_stats; + mutable Firebird::Mutex dbb_stats_mutex; + TraNumber dbb_last_header_write; // Transaction id of last header page physical write SLONG dbb_flush_cycle; // Current flush cycle ULONG dbb_sweep_interval; // Transactions between sweep diff --git a/src/jrd/JrdStatement.cpp b/src/jrd/JrdStatement.cpp index 5ba938babc..9d7dccbc76 100644 --- a/src/jrd/JrdStatement.cpp +++ b/src/jrd/JrdStatement.cpp @@ -281,8 +281,6 @@ JrdStatement* JrdStatement::makeStatement(thread_db* tdbb, CompilerScratch* csb, if (internalFlag) statement->flags |= FLAG_INTERNAL; - else - tdbb->bumpStats(RuntimeStatistics::STMT_PREPARES); return statement; } diff --git a/src/jrd/Monitoring.cpp b/src/jrd/Monitoring.cpp index 8f16970272..d714496165 100644 --- a/src/jrd/Monitoring.cpp +++ b/src/jrd/Monitoring.cpp @@ -786,6 +786,7 @@ void Monitoring::putDatabase(SnapshotData::DumpRecord& record, const Database* d if (database->dbb_flags & DBB_shared) { + MutexLockGuard guard(database->dbb_stats_mutex, FB_FUNCTION); putStatistics(record, database->dbb_stats, writer, stat_id, stat_database); putMemoryUsage(record, database->dbb_memory_stats, writer, stat_id, stat_database); } @@ -876,6 +877,7 @@ void Monitoring::putAttachment(SnapshotData::DumpRecord& record, const Jrd::Atta } else { + MutexLockGuard guard(attachment->att_database->dbb_stats_mutex, FB_FUNCTION); putStatistics(record, attachment->att_database->dbb_stats, writer, stat_id, stat_attachment); putMemoryUsage(record, attachment->att_database->dbb_memory_stats, writer, stat_id, stat_attachment); } @@ -1182,7 +1184,7 @@ void Monitoring::checkState(thread_db* tdbb) } -void Monitoring::dumpAttachment(thread_db* tdbb, const Attachment* attachment, bool ast) +void Monitoring::dumpAttachment(thread_db* tdbb, Attachment* attachment, bool ast) { if (!attachment->att_user) return; @@ -1190,6 +1192,8 @@ void Monitoring::dumpAttachment(thread_db* tdbb, const Attachment* attachment, b Database* const dbb = tdbb->getDatabase(); MemoryPool& pool = *dbb->dbb_permanent; + attachment->mergeStats(); + const AttNumber att_id = attachment->att_attachment_id; const string& user_name = attachment->att_user->usr_user_name; diff --git a/src/jrd/Monitoring.h b/src/jrd/Monitoring.h index ed4fd0729f..90ee300861 100644 --- a/src/jrd/Monitoring.h +++ b/src/jrd/Monitoring.h @@ -385,7 +385,7 @@ public: static void checkState(thread_db* tdbb); static SnapshotData* getSnapshot(thread_db* tdbb); - static void dumpAttachment(thread_db* tdbb, const Attachment* attachment, bool ast); + static void dumpAttachment(thread_db* tdbb, Attachment* attachment, bool ast); static void publishAttachment(thread_db* tdbb); static void cleanupAttachment(thread_db* tdbb); diff --git a/src/jrd/RuntimeStatistics.h b/src/jrd/RuntimeStatistics.h index 608253166e..1886ee8ff9 100644 --- a/src/jrd/RuntimeStatistics.h +++ b/src/jrd/RuntimeStatistics.h @@ -50,7 +50,6 @@ public: PAGE_READS, PAGE_MARKS, PAGE_WRITES, - FLUSHES, RECORD_SEQ_READS, RECORD_IDX_READS, RECORD_UPDATES, @@ -65,23 +64,18 @@ public: RECORD_BACKVERSION_READS, RECORD_FRAGMENT_READS, RECORD_RPT_READS, - SORTS, - SORT_GETS, - SORT_PUTS, - STMT_PREPARES, - STMT_EXECUTES, TOTAL_ITEMS // last }; +private: + static const size_t REL_BASE_OFFSET = RECORD_SEQ_READS; + static const size_t REL_TOTAL_ITEMS = RECORD_RPT_READS - REL_BASE_OFFSET + 1; + // Performance counters for individual table class RelationCounts { - static const size_t REL_BASE_OFFSET = RECORD_SEQ_READS; - public: - static const size_t REL_TOTAL_ITEMS = RECORD_RPT_READS - REL_BASE_OFFSET + 1; - explicit RelationCounts(SLONG relation_id) : rlc_relation_id(relation_id) { @@ -158,6 +152,7 @@ public: typedef Firebird::SortedArray, SLONG, RelationCounts> RelCounters; +public: RuntimeStatistics() : Firebird::AutoStorage(), rel_counts(getPool()) { @@ -226,12 +221,14 @@ public: // add difference between newStats and baseStats to our counters // newStats and baseStats must be "in-sync" - void adjust(const RuntimeStatistics& baseStats, const RuntimeStatistics& newStats) + void adjust(const RuntimeStatistics& baseStats, const RuntimeStatistics& newStats, bool relStatsOnly = false) { if (baseStats.allChgNumber != newStats.allChgNumber) { + const size_t FIRST_ITEM = relStatsOnly ? REL_BASE_OFFSET : 0; + allChgNumber++; - for (size_t i = 0; i < TOTAL_ITEMS; ++i) + for (size_t i = FIRST_ITEM; i < TOTAL_ITEMS; ++i) values[i] += newStats.values[i] - baseStats.values[i]; if (baseStats.relChgNumber != newStats.relChgNumber) diff --git a/src/jrd/cch.cpp b/src/jrd/cch.cpp index e9c026f5db..909d8d9d13 100644 --- a/src/jrd/cch.cpp +++ b/src/jrd/cch.cpp @@ -1037,8 +1037,6 @@ void CCH_flush(thread_db* tdbb, USHORT flush_flag, TraNumber tra_number) if (backup_state == Ods::hdr_nbak_stalled || backup_state == Ods::hdr_nbak_merge) bm->flushDifference(tdbb); } - - tdbb->bumpStats(RuntimeStatistics::FLUSHES); } // take the opportunity when we know there are no pages diff --git a/src/jrd/exe.cpp b/src/jrd/exe.cpp index be2ad07487..3299f372ad 100644 --- a/src/jrd/exe.cpp +++ b/src/jrd/exe.cpp @@ -902,9 +902,6 @@ void EXE_start(thread_db* tdbb, jrd_req* request, jrd_tra* transaction) impure->vlu_flags = 0; } - if (statement->sqlText) - tdbb->bumpStats(RuntimeStatistics::STMT_EXECUTES); - request->req_src_line = 0; request->req_src_column = 0; diff --git a/src/jrd/jrd.cpp b/src/jrd/jrd.cpp index 215c8ccdde..1ca63c2dd3 100644 --- a/src/jrd/jrd.cpp +++ b/src/jrd/jrd.cpp @@ -6358,6 +6358,8 @@ static void release_attachment(thread_db* tdbb, Jrd::Attachment* attachment) attachment->deletePool(pool); } + attachment->mergeStats(); + // remove the attachment block from the dbb linked list Sync sync(&dbb->dbb_sync, "jrd.cpp: release_attachment"); sync.lock(SYNC_EXCLUSIVE); @@ -7399,17 +7401,13 @@ void thread_db::setDatabase(Database* val) const bool wasActive = database && (priorThread || nextThread || database->dbb_active_threads == this); if (wasActive) - { deactivate(); - } database = val; dbbStat = val ? &val->dbb_stats : RuntimeStatistics::getDummy(); if (wasActive) - { activate(); - } } } diff --git a/src/jrd/jrd.h b/src/jrd/jrd.h index 2f99b89a1a..fdf7100a72 100644 --- a/src/jrd/jrd.h +++ b/src/jrd/jrd.h @@ -459,12 +459,30 @@ public: void bumpRelStats(const RuntimeStatistics::StatType index, SLONG relation_id, SINT64 delta = 1) { - bumpStats(index, delta); + // We don't bump counters for dbbStat here, they're merged from attStats on demand - reqStat->bumpRelValue(index, relation_id, delta); - traStat->bumpRelValue(index, relation_id, delta); - attStat->bumpRelValue(index, relation_id, delta); - dbbStat->bumpRelValue(index, relation_id, delta); + reqStat->bumpValue(index, delta); + traStat->bumpValue(index, delta); + attStat->bumpValue(index, delta); + + const RuntimeStatistics* const dummyStat = RuntimeStatistics::getDummy(); + + // We expect that at least attStat is present (not a dummy object) + + fb_assert(attStat != dummyStat); + + // Relation statistics is a quite complex beast, so a conditional check + // does not hurt. It also allows to avoid races while accessing the static + // dummy object concurrently. + + if (reqStat != dummyStat) + reqStat->bumpRelValue(index, relation_id, delta); + + if (traStat != dummyStat) + traStat->bumpRelValue(index, relation_id, delta); + + if (attStat != dummyStat) + attStat->bumpRelValue(index, relation_id, delta); } ISC_STATUS checkCancelState(); diff --git a/src/jrd/sort.cpp b/src/jrd/sort.cpp index ffe0a63f55..856b390c74 100644 --- a/src/jrd/sort.cpp +++ b/src/jrd/sort.cpp @@ -342,8 +342,6 @@ void Sort::get(thread_db* tdbb, ULONG** record_address) { diddleKey((UCHAR*) record->sort_record_key, false); } - - tdbb->bumpStats(RuntimeStatistics::SORT_GETS); } catch (const BadAlloc&) { @@ -419,8 +417,6 @@ void Sort::put(thread_db* tdbb, ULONG** record_address) *m_next_pointer++ = reinterpret_cast(record->sr_sort_record.sort_record_key); m_records++; *record_address = (ULONG*) record->sr_sort_record.sort_record_key; - - tdbb->bumpStats(RuntimeStatistics::SORT_PUTS); } catch (const BadAlloc&) { @@ -463,7 +459,6 @@ void Sort::sort(thread_db* tdbb) sort(); m_next_pointer = m_first_pointer + 1; m_flags |= scb_sorted; - tdbb->bumpStats(RuntimeStatistics::SORTS); return; } @@ -619,7 +614,6 @@ void Sort::sort(thread_db* tdbb) sortRunsBySeek(run_count); m_flags |= scb_sorted; - tdbb->bumpStats(RuntimeStatistics::SORTS); } catch (const BadAlloc&) {