@abh3 commented on this pull request.

OK, I think we are down to one alternate proposal which you may or may not want to make and one that you agreed to but wasn't yet made. It's all in the new comments.

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;

I see your point. Maybe this should be parametrized so that everyone gets to specify their own version of anticipation.

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;

OK, I guess the easiest change is to make it the same value as the default check time. Didn't see the change here.

In src/XrdHttp/XrdHttpSecurity.cc:

> @@ -164,6 +165,7 @@ XrdHttpProtocol::HandleGridMap(XrdLink* lp)
       TRACEI(DEBUG, " Mapping name: '" << SecEntity.moninfo << "' --> " << bufname);
       if (SecEntity.name) free(SecEntity.name);
       SecEntity.name = strdup(bufname);
+      SecEntity.eaAPI->Add("gridmap.name", bufname, true);

I'm sure you looked at it until you fell asleep. Nonetheless, is there any way to fix this (i.e. adhere to the wanted order)? Now, let me understand this. The order basically says if the gridmap file has a map in it use it, if not use he one in the VOMS mapfile, and if it doesn't exists use the default. If that is the logic then swapping 2 and 3 should not make a difference because the default would be replaced by the VOMS mapping is it exists or the default would be kept. In the end, the same result. Now, please tell me it's not the way it works :-)

—
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.Message ID: <xrootd/xrootd/pull/1572/review/864439809@github.com>

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