 |
 |
@abh3 requested changes on this pull request.
In src/XrdVoms/XrdVomsMapfile.hh:
> std::string m_mapfile;
std::shared_ptr<const std::vector<MapfileEntry>> m_entries;
XrdHttpSecXtractor *m_xrdvoms{nullptr};
XrdSysError *m_edest{nullptr};
- std::atomic<time_t> m_last_update;
+ // Pipes to allow the main thread to communicate shutdown events to the maintenance
+ // thread, allowing for a clean shutdown.
+ int m_maintenance_pipe_r{-1};
+ int m_maintenance_pipe_w{-1};
+ int m_maintenance_thread_pipe_r{-1};
+ int m_maintenance_thread_pipe_w{-1};
+
+ // After success, how long to wait until the next mapfile check.
+ static constexpr unsigned m_update_interval = 30;
OK, so I gather that 30 seconds is not in the overkill territory. Frankly, I think it is and 60 or even 180 would be more appropriate.
In src/XrdVoms/XrdVomsMapfile.hh:
> std::string m_mapfile;
std::shared_ptr<const std::vector<MapfileEntry>> m_entries;
XrdHttpSecXtractor *m_xrdvoms{nullptr};
XrdSysError *m_edest{nullptr};
- std::atomic<time_t> m_last_update;
+ // Pipes to allow the main thread to communicate shutdown events to the maintenance
+ // thread, allowing for a clean shutdown.
+ int m_maintenance_pipe_r{-1};
+ int m_maintenance_pipe_w{-1};
+ int m_maintenance_thread_pipe_r{-1};
+ int m_maintenance_thread_pipe_w{-1};
+
+ // After success, how long to wait until the next mapfile check.
+ static constexpr unsigned m_update_interval = 30;
+ // After failure, how long to wait until the next mapfile check.
+ static constexpr unsigned m_update_interval_failure = 3;
Nice addition but where is the limit? So, if someone makes a mistake we will start checking this thing every three seconds? Now, that is rather bad because there is no feedback other than the log and we will be doing a rather frequent check that is likely to fail all the time because the admin went to sleep. I am very skeptical about this check. In fact, let's say the file disappears, so now we will get a message in the log every three seconds filling up the log until someone looks at it. I think we should just use the default refresh time. This is rather dangerous.
In src/XrdVoms/XrdVomsMapfile.cc:
> + return mapper->IsValid() ? mapper.get() : nullptr;
+}
+
+
+void *
+XrdVomsMapfile::MaintenanceThread(void *myself_raw)
+{
+ auto myself = static_cast<XrdVomsMapfile*>(myself_raw);
+
+ auto now = monotonic_time_s();
+ auto next_update = now + m_update_interval;
+ while (true) {
+ now = monotonic_time_s();
+ auto remaining = next_update - now;
+ struct pollfd fds;
+ fds.fd = myself->m_maintenance_pipe_r;
My take is this is only for the case when the object is destroyed and you want to stop the maintenance thread. OK, I get that but the object will never be destroyed as far as I can see. Once the mapfile is started it continues for the life of the server. So, I am confused as to what problem this is trying to solve.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Use REPLY-ALL to reply to list
To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1